From: Jan Kara <jack@suse.cz> To: Mel Gorman <mgorman@techsingularity.net> Cc: 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 2/8] mm, truncate: Do not check mapping for every page being truncated Date: Thu, 12 Oct 2017 14:15:27 +0200 [thread overview] Message-ID: <20171012121527.GA29293@quack2.suse.cz> (raw) In-Reply-To: <20171012093103.13412-3-mgorman@techsingularity.net> On Thu 12-10-17 10:30:57, Mel Gorman wrote: > During truncation, the mapping has already been checked for shmem and dax > so it's known that workingset_update_node is required. This patch avoids > the checks on mapping for each page being truncated. In all other cases, > a lookup helper is used to determine if workingset_update_node() needs > to be called. The one danger is that the API is slightly harder to use as > calling workingset_update_node directly without checking for dax or shmem > mappings could lead to surprises. However, the API rarely needs to be used > and hopefully the comment is enough to give people the hint. > > sparsetruncate (tiny) > 4.14.0-rc4 4.14.0-rc4 > oneirq-v1r1 pickhelper-v1r1 > Min Time 141.00 ( 0.00%) 140.00 ( 0.71%) > 1st-qrtle Time 142.00 ( 0.00%) 141.00 ( 0.70%) > 2nd-qrtle Time 142.00 ( 0.00%) 142.00 ( 0.00%) > 3rd-qrtle Time 143.00 ( 0.00%) 143.00 ( 0.00%) > Max-90% Time 144.00 ( 0.00%) 144.00 ( 0.00%) > Max-95% Time 147.00 ( 0.00%) 145.00 ( 1.36%) > Max-99% Time 195.00 ( 0.00%) 191.00 ( 2.05%) > Max Time 230.00 ( 0.00%) 205.00 ( 10.87%) > Amean Time 144.37 ( 0.00%) 143.82 ( 0.38%) > Stddev Time 10.44 ( 0.00%) 9.00 ( 13.74%) > Coeff Time 7.23 ( 0.00%) 6.26 ( 13.41%) > Best99%Amean Time 143.72 ( 0.00%) 143.34 ( 0.26%) > Best95%Amean Time 142.37 ( 0.00%) 142.00 ( 0.26%) > Best90%Amean Time 142.19 ( 0.00%) 141.85 ( 0.24%) > Best75%Amean Time 141.92 ( 0.00%) 141.58 ( 0.24%) > Best50%Amean Time 141.69 ( 0.00%) 141.31 ( 0.27%) > Best25%Amean Time 141.38 ( 0.00%) 140.97 ( 0.29%) > > As you'd expect, the gain is marginal but it can be detected. The differences > in bonnie are all within the noise which is not surprising given the impact > on the microbenchmark. > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> ... > diff --git a/mm/workingset.c b/mm/workingset.c > index 7119cd745ace..a80d52387734 100644 > --- a/mm/workingset.c > +++ b/mm/workingset.c > @@ -341,12 +341,6 @@ static struct list_lru shadow_nodes; > > void workingset_update_node(struct radix_tree_node *node, void *private) > { > - struct address_space *mapping = private; > - > - /* Only regular page cache has shadow entries */ > - if (dax_mapping(mapping) || shmem_mapping(mapping)) > - return; > - Hum, we don't need to pass 'mapping' from call sites then? Either pass NULL or just remove the argument completely since nobody needs it anymore... Otherwise the patch looks good. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz> To: Mel Gorman <mgorman@techsingularity.net> Cc: 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 2/8] mm, truncate: Do not check mapping for every page being truncated Date: Thu, 12 Oct 2017 14:15:27 +0200 [thread overview] Message-ID: <20171012121527.GA29293@quack2.suse.cz> (raw) In-Reply-To: <20171012093103.13412-3-mgorman@techsingularity.net> On Thu 12-10-17 10:30:57, Mel Gorman wrote: > During truncation, the mapping has already been checked for shmem and dax > so it's known that workingset_update_node is required. This patch avoids > the checks on mapping for each page being truncated. In all other cases, > a lookup helper is used to determine if workingset_update_node() needs > to be called. The one danger is that the API is slightly harder to use as > calling workingset_update_node directly without checking for dax or shmem > mappings could lead to surprises. However, the API rarely needs to be used > and hopefully the comment is enough to give people the hint. > > sparsetruncate (tiny) > 4.14.0-rc4 4.14.0-rc4 > oneirq-v1r1 pickhelper-v1r1 > Min Time 141.00 ( 0.00%) 140.00 ( 0.71%) > 1st-qrtle Time 142.00 ( 0.00%) 141.00 ( 0.70%) > 2nd-qrtle Time 142.00 ( 0.00%) 142.00 ( 0.00%) > 3rd-qrtle Time 143.00 ( 0.00%) 143.00 ( 0.00%) > Max-90% Time 144.00 ( 0.00%) 144.00 ( 0.00%) > Max-95% Time 147.00 ( 0.00%) 145.00 ( 1.36%) > Max-99% Time 195.00 ( 0.00%) 191.00 ( 2.05%) > Max Time 230.00 ( 0.00%) 205.00 ( 10.87%) > Amean Time 144.37 ( 0.00%) 143.82 ( 0.38%) > Stddev Time 10.44 ( 0.00%) 9.00 ( 13.74%) > Coeff Time 7.23 ( 0.00%) 6.26 ( 13.41%) > Best99%Amean Time 143.72 ( 0.00%) 143.34 ( 0.26%) > Best95%Amean Time 142.37 ( 0.00%) 142.00 ( 0.26%) > Best90%Amean Time 142.19 ( 0.00%) 141.85 ( 0.24%) > Best75%Amean Time 141.92 ( 0.00%) 141.58 ( 0.24%) > Best50%Amean Time 141.69 ( 0.00%) 141.31 ( 0.27%) > Best25%Amean Time 141.38 ( 0.00%) 140.97 ( 0.29%) > > As you'd expect, the gain is marginal but it can be detected. The differences > in bonnie are all within the noise which is not surprising given the impact > on the microbenchmark. > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> ... > diff --git a/mm/workingset.c b/mm/workingset.c > index 7119cd745ace..a80d52387734 100644 > --- a/mm/workingset.c > +++ b/mm/workingset.c > @@ -341,12 +341,6 @@ static struct list_lru shadow_nodes; > > void workingset_update_node(struct radix_tree_node *node, void *private) > { > - struct address_space *mapping = private; > - > - /* Only regular page cache has shadow entries */ > - if (dax_mapping(mapping) || shmem_mapping(mapping)) > - return; > - Hum, we don't need to pass 'mapping' from call sites then? Either pass NULL or just remove the argument completely since nobody needs it anymore... Otherwise the patch looks good. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR -- 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-12 12:15 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-12 9:30 [PATCH 0/8] Follow-up for speed up page cache truncation Mel Gorman 2017-10-12 9:30 ` 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 2017-10-12 9:30 ` [PATCH 2/8] mm, truncate: Do not check mapping for every page being truncated Mel Gorman 2017-10-12 9:30 ` Mel Gorman 2017-10-12 12:15 ` Jan Kara [this message] 2017-10-12 12:15 ` Jan Kara 2017-10-12 12:41 ` Mel Gorman 2017-10-12 12:41 ` Mel Gorman 2017-10-12 19:11 ` Johannes Weiner 2017-10-12 19:11 ` Johannes Weiner 2017-10-12 9:30 ` [PATCH 3/8] mm, truncate: Remove all exceptional entries from pagevec under one lock Mel Gorman 2017-10-12 9:30 ` Mel Gorman 2017-10-12 13:33 ` Jan Kara 2017-10-12 13:33 ` Jan Kara 2017-10-12 14:53 ` Mel Gorman 2017-10-12 14:53 ` Mel Gorman 2017-10-12 19:45 ` Johannes Weiner 2017-10-12 19:45 ` Johannes Weiner 2017-10-12 9:30 ` [PATCH 4/8] mm: Only drain per-cpu pagevecs once per pagevec usage Mel Gorman 2017-10-12 9:30 ` Mel Gorman 2017-10-12 9:31 ` [PATCH 5/8] mm, pagevec: Remove cold parameter for pagevecs Mel Gorman 2017-10-12 9:31 ` Mel Gorman 2017-10-12 9:31 ` [PATCH 6/8] mm: Remove cold parameter for release_pages Mel Gorman 2017-10-12 9:31 ` [PATCH 7/8] mm, Remove cold parameter from free_hot_cold_page* Mel Gorman 2017-10-12 9:31 ` Mel Gorman 2017-10-12 9:31 ` [PATCH 8/8] mm: Remove __GFP_COLD Mel Gorman 2017-10-12 9:31 ` Mel Gorman 2017-10-18 7:59 [PATCH 0/8] Follow-up for speed up page cache truncation v2 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
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=20171012121527.GA29293@quack2.suse.cz \ --to=jack@suse.cz \ --cc=ak@linux.intel.com \ --cc=dave.hansen@intel.com \ --cc=david@fromorbit.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@techsingularity.net \ /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.