| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1210700635.6208.40.camel@lts-notebook>
Date: Tue, 13 May 2008 13:43:55 -0400
From: Lee Schermerhorn <Lee.Schermerhorn@...com>
To: Rik van Riel <riel@...hat.com>
Cc: Daisuke Nishimura <nishimura@....nes.nec.co.jp>,
akpm@...ux-foundation.org, kosaki.motohiro@...fujitsu.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] take pageout refcount into account for
remove_exclusive_swap_page
On Tue, 2008-05-13 at 12:04 -0400, Rik van Riel wrote:
> The pageout code takes a reference count to the page, which means
> that remove_exclusive_swap_page (when called from the pageout code)
> needs to take that extra refcount into account for mapped pages.
>
> Introduces a remove_exclusive_swap_page_ref function to avoid
> exposing too much magic to the rest of the VM.
>
> Signed-off-by: Rik van Riel <riel@...hat.com>
> Debugged-by: Daisuke Nishimura <nishimura@....nes.nec.co.jp>
>
> ---
> Daisuke: thank you for pointing out the problem. Does this patch look like
> a reasonable fix to you?
>
> Andrew: this patch is incremental to my patch
> "[PATCH -mm 05/15] free swap space on swap-in/activation"
>
>
> Index: linux-2.6.25-mm1/mm/vmscan.c
> ===================================================================
> --- linux-2.6.25-mm1.orig/mm/vmscan.c 2008-05-13 11:59:34.000000000 -0400
> +++ linux-2.6.25-mm1/mm/vmscan.c 2008-05-13 11:54:03.000000000 -0400
> @@ -621,7 +621,7 @@ free_it:
> activate_locked:
> /* Not a candidate for swapping, so reclaim swap space. */
> if (PageSwapCache(page) && vm_swap_full())
> - remove_exclusive_swap_page(page);
> + remove_exclusive_swap_page_ref(page);
> SetPageActive(page);
> pgactivate++;
> keep_locked:
> Index: linux-2.6.25-mm1/mm/swap.c
> ===================================================================
> --- linux-2.6.25-mm1.orig/mm/swap.c 2008-05-13 11:59:34.000000000 -0400
> +++ linux-2.6.25-mm1/mm/swap.c 2008-05-13 11:53:47.000000000 -0400
> @@ -455,7 +455,7 @@ void pagevec_swap_free(struct pagevec *p
>
> if (PageSwapCache(page) && !TestSetPageLocked(page)) {
> if (PageSwapCache(page))
> - remove_exclusive_swap_page(page);
> + remove_exclusive_swap_page_ref(page);
> unlock_page(page);
> }
> }
> Index: linux-2.6.25-mm1/include/linux/swap.h
> ===================================================================
> --- linux-2.6.25-mm1.orig/include/linux/swap.h 2008-05-13 11:20:46.000000000 -0400
> +++ linux-2.6.25-mm1/include/linux/swap.h 2008-05-13 11:47:31.000000000 -0400
> @@ -266,6 +266,7 @@ extern sector_t swapdev_block(int, pgoff
> extern struct swap_info_struct *get_swap_info_struct(unsigned);
> extern int can_share_swap_page(struct page *);
> extern int remove_exclusive_swap_page(struct page *);
> +extern int remove_exclusive_swap_page_ref(struct page *);
> struct backing_dev_info;
>
> extern spinlock_t swap_lock;
> Index: linux-2.6.25-mm1/mm/swapfile.c
> ===================================================================
> --- linux-2.6.25-mm1.orig/mm/swapfile.c 2008-04-22 10:33:45.000000000 -0400
> +++ linux-2.6.25-mm1/mm/swapfile.c 2008-05-13 11:53:32.000000000 -0400
> @@ -343,7 +343,7 @@ int can_share_swap_page(struct page *pag
> * Work out if there are any other processes sharing this
> * swap cache page. Free it if you can. Return success.
> */
> -int remove_exclusive_swap_page(struct page *page)
> +static int remove_exclusive_swap_page_count(struct page *page, int count)
> {
> int retval;
> struct swap_info_struct * p;
> @@ -356,7 +356,7 @@ int remove_exclusive_swap_page(struct pa
> return 0;
> if (PageWriteback(page))
> return 0;
> - if (page_count(page) != 2) /* 2: us + cache */
> + if (page_count(page) != count) /* 2: us + cache */
Maybe change comment to "/* count: us + ptes + cache */" ???
> return 0;
>
> entry.val = page_private(page);
> @@ -369,7 +369,7 @@ int remove_exclusive_swap_page(struct pa
> if (p->swap_map[swp_offset(entry)] == 1) {
> /* Recheck the page count with the swapcache lock held.. */
> write_lock_irq(&swapper_space.tree_lock);
> - if ((page_count(page) == 2) && !PageWriteback(page)) {
> + if ((page_count(page) == count) && !PageWriteback(page)) {
> __delete_from_swap_cache(page);
> SetPageDirty(page);
> retval = 1;
> @@ -387,6 +387,25 @@ int remove_exclusive_swap_page(struct pa
> }
>
> /*
> + * Most of the time the page should have two references: one for the
> + * process and one for the swap cache.
> + */
> +int remove_exclusive_swap_page(struct page *page)
> +{
> + return remove_exclusive_swap_page_count(page, 2);
> +}
> +
> +/*
> + * The pageout code holds an extra reference to the page. That raises
> + * the reference count to test for to 2 for a page that is only in the
> + * swap cache and 3 for a page that is mapped by a process.
Or, more generally, 2 + N, for an anon page that is mapped [must be
read-only, right?] by N processes. This can happen after, e.g., fork().
Looks like this patch handles the condition just fine, but you might
want to reflect this in the comment.
Now, I think I can use this to try to remove anon pages from the swap
cache when they're mlocked.
> + */
> +int remove_exclusive_swap_page_ref(struct page *page)
> +{
> + return remove_exclusive_swap_page_count(page, 2 + page_mapped(page));
> +}
> +
> +/*
> * Free the swap entry like above, but also try to
> * free the page cache entry if it is the last user.
> */
>
> All Rights Reversed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists