From: Mel Gorman <mgorman@suse.de> To: Michal Hocko <mhocko@kernel.org> Cc: linux-mm@kvack.org, Vlastimil Babka <vbabka@suse.cz>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, Andrew Morton <akpm@linux-foundation.org>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [RFC PATCH] mm: move pcp and lru-pcp drainging into vmstat_wq Date: Wed, 8 Feb 2017 12:31:13 +0000 [thread overview] Message-ID: <20170208123113.nq5unzmzpb23zoz5@suse.de> (raw) In-Reply-To: <20170208120354.GI5686@dhcp22.suse.cz> On Wed, Feb 08, 2017 at 01:03:55PM +0100, Michal Hocko wrote: > > I don't object to it being actually moved. I have a slight concern that > > it could somehow starve a vmstat update while frequent drains happen > > during reclaim though which potentially compounds the problem. It could > > be offset by a variety of other factors but if it ever is an issue, > > it'll show up and the paths that really matter check the vmstats > > directly instead of waiting for an update. > > vmstat updates can tolared delays, that's we we are using deferable > scheduling in the first place so I am not really worried about that. Any > user which needs a better precision should use *_snapshot API. > Agreed, we already had cases where deferred vmstat updates had problems and were resolved by using _snapshot. It's a slight concern only and I'd be surprised if the _snapshot usage didn't cover it. > > The altering of the return value in setup_vmstat was mildly surprising as > > it increases the severity of registering the vmstat callback for memory > > hotplug so maybe split that out and appears unrelated. > > not sure I understand. What do you mean? > This hunk @@ -1763,9 +1762,11 @@ static int vmstat_cpu_dead(unsigned int cpu) static int __init setup_vmstat(void) { -#ifdef CONFIG_SMP - int ret; + int ret = 0; + + vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0); +#ifdef CONFIG_SMP ret = cpuhp_setup_state_nocalls(CPUHP_MM_VMSTAT_DEAD, "mm/vmstat:dead", NULL, vmstat_cpu_dead); if (ret < 0) @@ -1789,7 +1790,7 @@ static int __init setup_vmstat(void) proc_create("vmstat", S_IRUGO, NULL, &proc_vmstat_file_operations); proc_create("zoneinfo", S_IRUGO, NULL, &proc_zoneinfo_file_operations); #endif - return 0; + return ret; A failed register of vmstat_cpu_dead is returning the failure code in an init function now. Chances are it'll never hit but it didn't seem related to the patches general intent. > > It also feels like vmstat is now a misleading name for something that > > handles vmstat, lru drains and per-cpu drains but that's cosmetic. > > yeah a better name sounds like a good thing. mm_nonblock_wq? > it's not always non-blocking. Maybe mm_percpu_wq to describev a workqueue that handles a variety of MM-related per-cpu updates? -- Mel Gorman SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@suse.de> To: Michal Hocko <mhocko@kernel.org> Cc: linux-mm@kvack.org, Vlastimil Babka <vbabka@suse.cz>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, Andrew Morton <akpm@linux-foundation.org>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [RFC PATCH] mm: move pcp and lru-pcp drainging into vmstat_wq Date: Wed, 8 Feb 2017 12:31:13 +0000 [thread overview] Message-ID: <20170208123113.nq5unzmzpb23zoz5@suse.de> (raw) In-Reply-To: <20170208120354.GI5686@dhcp22.suse.cz> On Wed, Feb 08, 2017 at 01:03:55PM +0100, Michal Hocko wrote: > > I don't object to it being actually moved. I have a slight concern that > > it could somehow starve a vmstat update while frequent drains happen > > during reclaim though which potentially compounds the problem. It could > > be offset by a variety of other factors but if it ever is an issue, > > it'll show up and the paths that really matter check the vmstats > > directly instead of waiting for an update. > > vmstat updates can tolared delays, that's we we are using deferable > scheduling in the first place so I am not really worried about that. Any > user which needs a better precision should use *_snapshot API. > Agreed, we already had cases where deferred vmstat updates had problems and were resolved by using _snapshot. It's a slight concern only and I'd be surprised if the _snapshot usage didn't cover it. > > The altering of the return value in setup_vmstat was mildly surprising as > > it increases the severity of registering the vmstat callback for memory > > hotplug so maybe split that out and appears unrelated. > > not sure I understand. What do you mean? > This hunk @@ -1763,9 +1762,11 @@ static int vmstat_cpu_dead(unsigned int cpu) static int __init setup_vmstat(void) { -#ifdef CONFIG_SMP - int ret; + int ret = 0; + + vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0); +#ifdef CONFIG_SMP ret = cpuhp_setup_state_nocalls(CPUHP_MM_VMSTAT_DEAD, "mm/vmstat:dead", NULL, vmstat_cpu_dead); if (ret < 0) @@ -1789,7 +1790,7 @@ static int __init setup_vmstat(void) proc_create("vmstat", S_IRUGO, NULL, &proc_vmstat_file_operations); proc_create("zoneinfo", S_IRUGO, NULL, &proc_zoneinfo_file_operations); #endif - return 0; + return ret; A failed register of vmstat_cpu_dead is returning the failure code in an init function now. Chances are it'll never hit but it didn't seem related to the patches general intent. > > It also feels like vmstat is now a misleading name for something that > > handles vmstat, lru drains and per-cpu drains but that's cosmetic. > > yeah a better name sounds like a good thing. mm_nonblock_wq? > it's not always non-blocking. Maybe mm_percpu_wq to describev a workqueue that handles a variety of MM-related per-cpu updates? -- Mel Gorman SUSE Labs -- 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-02-08 12:43 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-02-07 21:09 [RFC PATCH] mm: move pcp and lru-pcp drainging into vmstat_wq Michal Hocko 2017-02-07 21:09 ` Michal Hocko 2017-02-08 10:49 ` Vlastimil Babka 2017-02-08 10:49 ` Vlastimil Babka 2017-02-08 11:56 ` Michal Hocko 2017-02-08 11:56 ` Michal Hocko 2017-02-08 10:53 ` Mel Gorman 2017-02-08 10:53 ` Mel Gorman 2017-02-08 12:03 ` Michal Hocko 2017-02-08 12:03 ` Michal Hocko 2017-02-08 12:31 ` Mel Gorman [this message] 2017-02-08 12:31 ` Mel Gorman 2017-02-08 12:44 ` Tetsuo Handa 2017-02-08 12:44 ` Tetsuo Handa 2017-02-08 12:56 ` Michal Hocko 2017-02-08 12:56 ` Michal Hocko 2017-02-08 12:50 ` Michal Hocko 2017-02-08 12:50 ` Michal Hocko
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=20170208123113.nq5unzmzpb23zoz5@suse.de \ --to=mgorman@suse.de \ --cc=akpm@linux-foundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mhocko@kernel.org \ --cc=penguin-kernel@I-love.SAKURA.ne.jp \ --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.