From: Mel Gorman <mgorman@techsingularity.net> To: Vlastimil Babka <vbabka@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org>, Linux-MM <linux-mm@kvack.org>, Linux-FSDevel <linux-fsdevel@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, Jan Kara <jack@suse.cz>, Andi Kleen <ak@linux.intel.com>, Dave Hansen <dave.hansen@intel.com>, Dave Chinner <david@fromorbit.com> Subject: Re: [PATCH 1/8] mm, page_alloc: Enable/disable IRQs once when freeing a list of pages Date: Wed, 18 Oct 2017 11:15:47 +0100 [thread overview] Message-ID: <20171018101547.mjycw7zreb66jzpa@techsingularity.net> (raw) In-Reply-To: <bcd95a87-3f63-9f5d-77a0-2b2115f53919@suse.cz> On Wed, Oct 18, 2017 at 11:02:18AM +0200, Vlastimil Babka wrote: > On 10/18/2017 09:59 AM, Mel Gorman wrote: > > Freeing a list of pages current enables/disables IRQs for each page freed. > > This patch splits freeing a list of pages into two operations -- preparing > > the pages for freeing and the actual freeing. This is a tradeoff - we're > > taking two passes of the list to free in exchange for avoiding multiple > > enable/disable of IRQs. > > There's also some overhead of storing pfn in page->private, but all that > seems negligible compared to irq disable/enable... > Exactly and it's cheaper than doing a second page to pfn lookup. > <SNIP> > Looks good. > > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > Thanks. > A nit below. > > > @@ -2647,11 +2663,25 @@ void free_hot_cold_page(struct page *page, bool cold) > > void free_hot_cold_page_list(struct list_head *list, bool cold) > > { > > struct page *page, *next; > > + unsigned long flags, pfn; > > + > > + /* Prepare pages for freeing */ > > + list_for_each_entry_safe(page, next, list, lru) { > > + pfn = page_to_pfn(page); > > + if (!free_hot_cold_page_prepare(page, pfn)) > > + list_del(&page->lru); > > + page->private = pfn; > > We have (set_)page_private() helpers so better to use them (makes it a > bit easier to check for all places where page->private is used to e.g. > avoid a clash)? > Agreed and it's trivial to do so ---8<--- mm, page_alloc: Enable/disable IRQs once when freeing a list of page -fix Use page_private and set_page_private helpers. Signed-off-by: Mel Gorman <mgorman@techsingularity.net> --- mm/page_alloc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 167e163cf733..092973014c1e 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2670,14 +2670,14 @@ void free_hot_cold_page_list(struct list_head *list, bool cold) pfn = page_to_pfn(page); if (!free_hot_cold_page_prepare(page, pfn)) list_del(&page->lru); - page->private = pfn; + set_page_private(page, pfn); } local_irq_save(flags); list_for_each_entry_safe(page, next, list, lru) { - unsigned long pfn = page->private; + unsigned long pfn = page_private(page); - page->private = 0; + set_page_private(page, 0); trace_mm_page_free_batched(page, cold); free_hot_cold_page_commit(page, pfn, cold); }
WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@techsingularity.net> To: Vlastimil Babka <vbabka@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org>, Linux-MM <linux-mm@kvack.org>, Linux-FSDevel <linux-fsdevel@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, Jan Kara <jack@suse.cz>, Andi Kleen <ak@linux.intel.com>, Dave Hansen <dave.hansen@intel.com>, Dave Chinner <david@fromorbit.com> Subject: Re: [PATCH 1/8] mm, page_alloc: Enable/disable IRQs once when freeing a list of pages Date: Wed, 18 Oct 2017 11:15:47 +0100 [thread overview] Message-ID: <20171018101547.mjycw7zreb66jzpa@techsingularity.net> (raw) In-Reply-To: <bcd95a87-3f63-9f5d-77a0-2b2115f53919@suse.cz> On Wed, Oct 18, 2017 at 11:02:18AM +0200, Vlastimil Babka wrote: > On 10/18/2017 09:59 AM, Mel Gorman wrote: > > Freeing a list of pages current enables/disables IRQs for each page freed. > > This patch splits freeing a list of pages into two operations -- preparing > > the pages for freeing and the actual freeing. This is a tradeoff - we're > > taking two passes of the list to free in exchange for avoiding multiple > > enable/disable of IRQs. > > There's also some overhead of storing pfn in page->private, but all that > seems negligible compared to irq disable/enable... > Exactly and it's cheaper than doing a second page to pfn lookup. > <SNIP> > Looks good. > > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > Thanks. > A nit below. > > > @@ -2647,11 +2663,25 @@ void free_hot_cold_page(struct page *page, bool cold) > > void free_hot_cold_page_list(struct list_head *list, bool cold) > > { > > struct page *page, *next; > > + unsigned long flags, pfn; > > + > > + /* Prepare pages for freeing */ > > + list_for_each_entry_safe(page, next, list, lru) { > > + pfn = page_to_pfn(page); > > + if (!free_hot_cold_page_prepare(page, pfn)) > > + list_del(&page->lru); > > + page->private = pfn; > > We have (set_)page_private() helpers so better to use them (makes it a > bit easier to check for all places where page->private is used to e.g. > avoid a clash)? > Agreed and it's trivial to do so ---8<--- mm, page_alloc: Enable/disable IRQs once when freeing a list of page -fix Use page_private and set_page_private helpers. Signed-off-by: Mel Gorman <mgorman@techsingularity.net> --- mm/page_alloc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 167e163cf733..092973014c1e 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2670,14 +2670,14 @@ void free_hot_cold_page_list(struct list_head *list, bool cold) pfn = page_to_pfn(page); if (!free_hot_cold_page_prepare(page, pfn)) list_del(&page->lru); - page->private = pfn; + set_page_private(page, pfn); } local_irq_save(flags); list_for_each_entry_safe(page, next, list, lru) { - unsigned long pfn = page->private; + unsigned long pfn = page_private(page); - page->private = 0; + set_page_private(page, 0); trace_mm_page_free_batched(page, cold); free_hot_cold_page_commit(page, pfn, cold); } -- 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>
next prev parent reply other threads:[~2017-10-18 10:15 UTC|newest] Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-18 7:59 [PATCH 0/8] Follow-up for speed up page cache truncation v2 Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-18 7:59 ` [PATCH 1/8] mm, page_alloc: Enable/disable IRQs once when freeing a list of pages Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-18 9:02 ` Vlastimil Babka 2017-10-18 9:02 ` Vlastimil Babka 2017-10-18 10:15 ` Mel Gorman [this message] 2017-10-18 10:15 ` Mel Gorman 2017-10-18 7:59 ` [PATCH 2/8] mm, truncate: Do not check mapping for every page being truncated Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-19 8:11 ` Jan Kara 2017-10-19 8:11 ` Jan Kara 2017-10-18 7:59 ` [PATCH 3/8] mm, truncate: Remove all exceptional entries from pagevec under one lock Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-18 7:59 ` [PATCH 4/8] mm: Only drain per-cpu pagevecs once per pagevec usage Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-19 9:12 ` Vlastimil Babka 2017-10-19 9:12 ` Vlastimil Babka 2017-10-19 9:33 ` Mel Gorman 2017-10-19 9:33 ` Mel Gorman 2017-10-19 13:21 ` Vlastimil Babka 2017-10-19 13:21 ` Vlastimil Babka 2017-10-18 7:59 ` [PATCH 5/8] mm, pagevec: Remove cold parameter for pagevecs Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-19 9:24 ` Vlastimil Babka 2017-10-19 9:24 ` Vlastimil Babka 2017-10-18 7:59 ` [PATCH 6/8] mm: Remove cold parameter for release_pages Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-19 9:26 ` Vlastimil Babka 2017-10-19 9:26 ` Vlastimil Babka 2017-10-18 7:59 ` [PATCH 7/8] mm, Remove cold parameter from free_hot_cold_page* Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-19 13:12 ` Vlastimil Babka 2017-10-19 13:12 ` Vlastimil Babka 2017-10-19 15:43 ` Mel Gorman 2017-10-19 15:43 ` Mel Gorman 2017-10-18 7:59 ` [PATCH 8/8] mm: Remove __GFP_COLD Mel Gorman 2017-10-18 7:59 ` Mel Gorman 2017-10-19 13:18 ` Vlastimil Babka 2017-10-19 13:18 ` Vlastimil Babka 2017-10-19 13:42 ` Vlastimil Babka 2017-10-19 13:42 ` Vlastimil Babka 2017-10-19 13:42 ` Vlastimil Babka 2017-10-19 14:32 ` Mel Gorman 2017-10-19 14:32 ` Mel Gorman -- strict thread matches above, loose matches on Subject: below -- 2017-10-12 9:30 [PATCH 0/8] Follow-up for speed up page cache truncation Mel Gorman 2017-10-12 9:30 ` [PATCH 1/8] mm, page_alloc: Enable/disable IRQs once when freeing a list of pages Mel Gorman 2017-10-12 9:30 ` Mel Gorman
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=20171018101547.mjycw7zreb66jzpa@techsingularity.net \ --to=mgorman@techsingularity.net \ --cc=ak@linux.intel.com \ --cc=akpm@linux-foundation.org \ --cc=dave.hansen@intel.com \ --cc=david@fromorbit.com \ --cc=jack@suse.cz \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=vbabka@suse.cz \ /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: linkBe 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.