From: Aaron Lu <aaron.lu@intel.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Mel Gorman <mgorman@techsingularity.net>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Huang Ying <ying.huang@intel.com>,
Dave Hansen <dave.hansen@intel.com>,
Kemi Wang <kemi.wang@intel.com>,
Tim Chen <tim.c.chen@linux.intel.com>,
Andi Kleen <ak@linux.intel.com>, Michal Hocko <mhocko@suse.com>,
Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [PATCH v2 1/2] free_pcppages_bulk: do not hold lock when picking pages to free
Date: Fri, 23 Feb 2018 09:42:46 +0800 [thread overview]
Message-ID: <20180223014245.GB4338@intel.com> (raw)
In-Reply-To: <20180215124644.GA12360@bombadil.infradead.org>
On Thu, Feb 15, 2018 at 04:46:44AM -0800, Matthew Wilcox wrote:
> On Thu, Jan 25, 2018 at 03:21:44PM +0800, Aaron Lu wrote:
> > When freeing a batch of pages from Per-CPU-Pages(PCP) back to buddy,
> > the zone->lock is held and then pages are chosen from PCP's migratetype
> > list. While there is actually no need to do this 'choose part' under
> > lock since it's PCP pages, the only CPU that can touch them is us and
> > irq is also disabled.
>
> I have no objection to this patch. If you're looking for ideas for
> future improvement though, I wonder whether using a LIST_HEAD is the
> best way to store these pages temporarily. If you batch them into a
> pagevec and then free the entire pagevec, the CPU should be a little
> faster scanning a short array than walking a linked list.
Thanks for the suggestion.
> It would also puts a hard boundary on how long zone->lock is held, as
> you'd drop it and go back for another batch after 15 pages. That might
> be bad, of course.
Yes that's a concern.
As Mel reponded in another email, I think I'll just keep using list
here.
>
> Another minor change I'd like to see is free_pcpages_bulk updating
> pcp->count itself; all of the callers do it currently.
Sounds good, I'll prepare a separate patch for this, thanks!
--
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: Aaron Lu <aaron.lu@intel.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Mel Gorman <mgorman@techsingularity.net>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Huang Ying <ying.huang@intel.com>,
Dave Hansen <dave.hansen@intel.com>,
Kemi Wang <kemi.wang@intel.com>,
Tim Chen <tim.c.chen@linux.intel.com>,
Andi Kleen <ak@linux.intel.com>, Michal Hocko <mhocko@suse.com>,
Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [PATCH v2 1/2] free_pcppages_bulk: do not hold lock when picking pages to free
Date: Fri, 23 Feb 2018 09:42:46 +0800 [thread overview]
Message-ID: <20180223014245.GB4338@intel.com> (raw)
In-Reply-To: <20180215124644.GA12360@bombadil.infradead.org>
On Thu, Feb 15, 2018 at 04:46:44AM -0800, Matthew Wilcox wrote:
> On Thu, Jan 25, 2018 at 03:21:44PM +0800, Aaron Lu wrote:
> > When freeing a batch of pages from Per-CPU-Pages(PCP) back to buddy,
> > the zone->lock is held and then pages are chosen from PCP's migratetype
> > list. While there is actually no need to do this 'choose part' under
> > lock since it's PCP pages, the only CPU that can touch them is us and
> > irq is also disabled.
>
> I have no objection to this patch. If you're looking for ideas for
> future improvement though, I wonder whether using a LIST_HEAD is the
> best way to store these pages temporarily. If you batch them into a
> pagevec and then free the entire pagevec, the CPU should be a little
> faster scanning a short array than walking a linked list.
Thanks for the suggestion.
> It would also puts a hard boundary on how long zone->lock is held, as
> you'd drop it and go back for another batch after 15 pages. That might
> be bad, of course.
Yes that's a concern.
As Mel reponded in another email, I think I'll just keep using list
here.
>
> Another minor change I'd like to see is free_pcpages_bulk updating
> pcp->count itself; all of the callers do it currently.
Sounds good, I'll prepare a separate patch for this, thanks!
next prev parent reply other threads:[~2018-02-23 1:41 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-24 2:30 [PATCH 1/2] free_pcppages_bulk: do not hold lock when picking pages to free Aaron Lu
2018-01-24 2:30 ` Aaron Lu
2018-01-24 2:30 ` [PATCH 2/2] free_pcppages_bulk: prefetch buddy while not holding lock Aaron Lu
2018-01-24 2:30 ` Aaron Lu
2018-01-24 16:43 ` Mel Gorman
2018-01-24 16:43 ` Mel Gorman
2018-01-24 16:57 ` Dave Hansen
2018-01-24 16:57 ` Dave Hansen
2018-01-24 18:19 ` Mel Gorman
2018-01-24 18:19 ` Mel Gorman
2018-01-24 19:23 ` Dave Hansen
2018-01-24 19:23 ` Dave Hansen
2018-01-24 21:12 ` Mel Gorman
2018-01-24 21:12 ` Mel Gorman
2018-01-25 7:25 ` [PATCH v2 " Aaron Lu
2018-01-25 7:25 ` Aaron Lu
2018-01-24 16:40 ` [PATCH 1/2] free_pcppages_bulk: do not hold lock when picking pages to free Mel Gorman
2018-01-24 16:40 ` Mel Gorman
2018-01-25 7:21 ` [PATCH v2 " Aaron Lu
2018-01-25 7:21 ` Aaron Lu
2018-02-15 12:06 ` Mel Gorman
2018-02-15 12:06 ` Mel Gorman
2018-02-23 1:37 ` Aaron Lu
2018-02-23 1:37 ` Aaron Lu
2018-02-15 12:46 ` Matthew Wilcox
2018-02-15 12:46 ` Matthew Wilcox
2018-02-15 14:55 ` Mel Gorman
2018-02-15 14:55 ` Mel Gorman
2018-02-23 1:42 ` Aaron Lu [this message]
2018-02-23 1:42 ` Aaron Lu
2018-02-05 5:30 ` RFC: eliminate zone->lock contention for will-it-scale/page_fault1 on big server Aaron Lu
2018-02-05 5:30 ` Aaron Lu
2018-02-05 5:31 ` [RFC PATCH 1/2] __free_one_page: skip merge for order-0 page unless compaction is in progress Aaron Lu
2018-02-05 5:31 ` Aaron Lu
2018-02-05 22:17 ` Dave Hansen
2018-02-05 22:17 ` Dave Hansen
2018-02-05 5:32 ` [RFC PATCH 2/2] rmqueue_bulk: avoid touching page structures under zone->lock Aaron Lu
2018-02-05 5:32 ` Aaron Lu
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=20180223014245.GB4338@intel.com \
--to=aaron.lu@intel.com \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=dave.hansen@intel.com \
--cc=kemi.wang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@techsingularity.net \
--cc=mhocko@suse.com \
--cc=tim.c.chen@linux.intel.com \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
--cc=ying.huang@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.