From: Dave Hansen <dave@sr71.net>
To: Minchan Kim <minchan@kernel.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
akpm@linux-foundation.org, mgorman@suse.de,
tim.c.chen@linux.intel.com
Subject: Re: [v4][PATCH 2/6] mm: swap: make 'struct page' and swp_entry_t variants of swapcache_free().
Date: Mon, 03 Jun 2013 08:55:08 -0700 [thread overview]
Message-ID: <51ACBC5C.9020701@sr71.net> (raw)
In-Reply-To: <20130603061320.GA2795@blaptop>
On 06/02/2013 11:13 PM, Minchan Kim wrote:
> I lost from this description.
>
> Old behavior
>
> delete_from_swap_cache
> swap_address_space
> __delete_from_swap_cache
> swap_address_space
>
>
> New behavior
>
> delete_from_swap_cache
> __delete_from_swap_cache
> swap_address_space
>
> So you removes a swap_address_space, not adding a extra call.
> Am I missing something?
I think I got the page->swp_entry_t lookup confused with the
page->swap_address_space lookup when I was writing the description. The
bit that you missed is that I _added_ a page_mapping() call, which calls
swap_address_space() internally:
Old behavior:
delete_from_swap_cache
swap_address_space
__delete_from_swap_cache
swap_address_space
New behavior:
delete_from_swap_cache
page_mapping
swap_address_space
__delete_from_swap_cache
swap_address_space
--
New description (last paragraph changed). Andrew, I'll resend the
series since there are a few of these cleanups.
From: Dave Hansen <dave.hansen@linux.intel.com>
swapcache_free() takes two arguments:
void swapcache_free(swp_entry_t entry, struct page *page)
Most of its callers (5/7) are from error handling paths haven't even
instantiated a page, so they pass page=NULL. Both of the callers
that call in with a 'struct page' create and pass in a temporary
swp_entry_t.
Now that we are deferring clearing page_private() until after
swapcache_free() has been called, we can just create a variant
that takes a 'struct page' and does the temporary variable in
the helper.
That leaves all the other callers doing
swapcache_free(entry, NULL)
so create another helper for them that makes it clear that they
need only pass in a swp_entry_t.
One downside here is that delete_from_swap_cache() now calls
swap_address_space() via page_mapping() instead of calling
swap_address_space() directly. In doing so, it removes one more
case of the swap cache code being special-cased, which is a good
thing in my book.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave@sr71.net>
To: Minchan Kim <minchan@kernel.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
akpm@linux-foundation.org, mgorman@suse.de,
tim.c.chen@linux.intel.com
Subject: Re: [v4][PATCH 2/6] mm: swap: make 'struct page' and swp_entry_t variants of swapcache_free().
Date: Mon, 03 Jun 2013 08:55:08 -0700 [thread overview]
Message-ID: <51ACBC5C.9020701@sr71.net> (raw)
In-Reply-To: <20130603061320.GA2795@blaptop>
On 06/02/2013 11:13 PM, Minchan Kim wrote:
> I lost from this description.
>
> Old behavior
>
> delete_from_swap_cache
> swap_address_space
> __delete_from_swap_cache
> swap_address_space
>
>
> New behavior
>
> delete_from_swap_cache
> __delete_from_swap_cache
> swap_address_space
>
> So you removes a swap_address_space, not adding a extra call.
> Am I missing something?
I think I got the page->swp_entry_t lookup confused with the
page->swap_address_space lookup when I was writing the description. The
bit that you missed is that I _added_ a page_mapping() call, which calls
swap_address_space() internally:
Old behavior:
delete_from_swap_cache
swap_address_space
__delete_from_swap_cache
swap_address_space
New behavior:
delete_from_swap_cache
page_mapping
swap_address_space
__delete_from_swap_cache
swap_address_space
--
New description (last paragraph changed). Andrew, I'll resend the
series since there are a few of these cleanups.
From: Dave Hansen <dave.hansen@linux.intel.com>
swapcache_free() takes two arguments:
void swapcache_free(swp_entry_t entry, struct page *page)
Most of its callers (5/7) are from error handling paths haven't even
instantiated a page, so they pass page=NULL. Both of the callers
that call in with a 'struct page' create and pass in a temporary
swp_entry_t.
Now that we are deferring clearing page_private() until after
swapcache_free() has been called, we can just create a variant
that takes a 'struct page' and does the temporary variable in
the helper.
That leaves all the other callers doing
swapcache_free(entry, NULL)
so create another helper for them that makes it clear that they
need only pass in a swp_entry_t.
One downside here is that delete_from_swap_cache() now calls
swap_address_space() via page_mapping() instead of calling
swap_address_space() directly. In doing so, it removes one more
case of the swap cache code being special-cased, which is a good
thing in my book.
next prev parent reply other threads:[~2013-06-03 15:55 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-31 18:38 [v4][PATCH 0/6] mm: vmscan: Batch page reclamation under shink_page_list Dave Hansen
2013-05-31 18:38 ` Dave Hansen
2013-05-31 18:38 ` [v4][PATCH 1/6] mm: swap: defer clearing of page_private() for swap cache pages Dave Hansen
2013-05-31 18:38 ` Dave Hansen
2013-06-03 5:40 ` Minchan Kim
2013-06-03 5:40 ` Minchan Kim
2013-06-03 14:53 ` Dave Hansen
2013-06-03 14:53 ` Dave Hansen
2013-06-04 4:41 ` Minchan Kim
2013-06-04 4:41 ` Minchan Kim
2013-05-31 18:38 ` [v4][PATCH 2/6] mm: swap: make 'struct page' and swp_entry_t variants of swapcache_free() Dave Hansen
2013-05-31 18:38 ` Dave Hansen
2013-06-03 6:13 ` Minchan Kim
2013-06-03 6:13 ` Minchan Kim
2013-06-03 15:55 ` Dave Hansen [this message]
2013-06-03 15:55 ` Dave Hansen
2013-05-31 18:38 ` [v4][PATCH 3/6] mm: vmscan: break up __remove_mapping() Dave Hansen
2013-05-31 18:38 ` Dave Hansen
2013-06-03 8:28 ` Minchan Kim
2013-06-03 8:28 ` Minchan Kim
2013-05-31 18:39 ` [v4][PATCH 4/6] mm: vmscan: break out mapping "freepage" code Dave Hansen
2013-05-31 18:39 ` Dave Hansen
2013-06-03 8:35 ` Minchan Kim
2013-06-03 8:35 ` Minchan Kim
2013-05-31 18:39 ` [v4][PATCH 5/6] mm: vmscan: batch shrink_page_list() locking operations Dave Hansen
2013-05-31 18:39 ` Dave Hansen
2013-05-31 18:39 ` [v4][PATCH 6/6] mm: vmscan: drain batch list during long operations Dave Hansen
2013-05-31 18:39 ` Dave Hansen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51ACBC5C.9020701@sr71.net \
--to=dave@sr71.net \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=minchan@kernel.org \
--cc=tim.c.chen@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.