From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933516AbdKPBlv (ORCPT ); Wed, 15 Nov 2017 20:41:51 -0500 Received: from mail-wr0-f193.google.com ([209.85.128.193]:51959 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754587AbdKPBln (ORCPT ); Wed, 15 Nov 2017 20:41:43 -0500 X-Google-Smtp-Source: AGs4zMbvmrXoa7UfAOrvO7oXVx4YkWAl3E0bKna1MQc1AhVkXy7v/r7TbA/ACC7ToUVjLBH70lgr1iQa1d+7KvhkQo8= MIME-Version: 1.0 In-Reply-To: <20171116004614.GB12222@bbox> References: <1510609063-3327-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> <20171115005602.GB23810@bbox> <20171116004614.GB12222@bbox> From: Shakeel Butt Date: Wed, 15 Nov 2017 17:41:41 -0800 Message-ID: Subject: Re: [PATCH 1/2] mm,vmscan: Kill global shrinker lock. To: Minchan Kim Cc: Tetsuo Handa , Huang Ying , Mel Gorman , Vladimir Davydov , Michal Hocko , Johannes Weiner , Andrew Morton , Greg Thelen , Linux MM , LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 15, 2017 at 4:46 PM, Minchan Kim wrote: > On Tue, Nov 14, 2017 at 10:28:10PM -0800, Shakeel Butt wrote: >> On Tue, Nov 14, 2017 at 4:56 PM, Minchan Kim wrote: >> > On Tue, Nov 14, 2017 at 06:37:42AM +0900, Tetsuo Handa wrote: >> >> When shrinker_rwsem was introduced, it was assumed that >> >> register_shrinker()/unregister_shrinker() are really unlikely paths >> >> which are called during initialization and tear down. But nowadays, >> >> register_shrinker()/unregister_shrinker() might be called regularly. >> >> This patch prepares for allowing parallel registration/unregistration >> >> of shrinkers. >> >> >> >> Since do_shrink_slab() can reschedule, we cannot protect shrinker_list >> >> using one RCU section. But using atomic_inc()/atomic_dec() for each >> >> do_shrink_slab() call will not impact so much. >> >> >> >> This patch uses polling loop with short sleep for unregister_shrinker() >> >> rather than wait_on_atomic_t(), for we can save reader's cost (plain >> >> atomic_dec() compared to atomic_dec_and_test()), we can expect that >> >> do_shrink_slab() of unregistering shrinker likely returns shortly, and >> >> we can avoid khungtaskd warnings when do_shrink_slab() of unregistering >> >> shrinker unexpectedly took so long. >> >> >> >> Signed-off-by: Tetsuo Handa >> > >> > Before reviewing this patch, can't we solve the problem with more >> > simple way? Like this. >> > >> > Shakeel, What do you think? >> > >> >> Seems simple enough. I will run my test (running fork bomb in one >> memcg and separately time a mount operation) and update if numbers >> differ significantly. > > Thanks. > >> >> > diff --git a/mm/vmscan.c b/mm/vmscan.c >> > index 13d711dd8776..cbb624cb9baa 100644 >> > --- a/mm/vmscan.c >> > +++ b/mm/vmscan.c >> > @@ -498,6 +498,14 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, >> > sc.nid = 0; >> > >> > freed += do_shrink_slab(&sc, shrinker, nr_scanned, nr_eligible); >> > + /* >> > + * bail out if someone want to register a new shrinker to prevent >> > + * long time stall by parallel ongoing shrinking. >> > + */ >> > + if (rwsem_is_contended(&shrinker_rwsem)) { >> > + freed = 1; >> >> freed = freed ?: 1; > > Yub. Thanks Minchan, you can add Reviewed-and-tested-by: Shakeel Butt From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f72.google.com (mail-wm0-f72.google.com [74.125.82.72]) by kanga.kvack.org (Postfix) with ESMTP id BD5E56B0277 for ; Wed, 15 Nov 2017 20:41:44 -0500 (EST) Received: by mail-wm0-f72.google.com with SMTP id b189so1550382wmd.5 for ; Wed, 15 Nov 2017 17:41:44 -0800 (PST) Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id y3sor7169351wrd.72.2017.11.15.17.41.42 for (Google Transport Security); Wed, 15 Nov 2017 17:41:43 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20171116004614.GB12222@bbox> References: <1510609063-3327-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> <20171115005602.GB23810@bbox> <20171116004614.GB12222@bbox> From: Shakeel Butt Date: Wed, 15 Nov 2017 17:41:41 -0800 Message-ID: Subject: Re: [PATCH 1/2] mm,vmscan: Kill global shrinker lock. Content-Type: text/plain; charset="UTF-8" Sender: owner-linux-mm@kvack.org List-ID: To: Minchan Kim Cc: Tetsuo Handa , Huang Ying , Mel Gorman , Vladimir Davydov , Michal Hocko , Johannes Weiner , Andrew Morton , Greg Thelen , Linux MM , LKML On Wed, Nov 15, 2017 at 4:46 PM, Minchan Kim wrote: > On Tue, Nov 14, 2017 at 10:28:10PM -0800, Shakeel Butt wrote: >> On Tue, Nov 14, 2017 at 4:56 PM, Minchan Kim wrote: >> > On Tue, Nov 14, 2017 at 06:37:42AM +0900, Tetsuo Handa wrote: >> >> When shrinker_rwsem was introduced, it was assumed that >> >> register_shrinker()/unregister_shrinker() are really unlikely paths >> >> which are called during initialization and tear down. But nowadays, >> >> register_shrinker()/unregister_shrinker() might be called regularly. >> >> This patch prepares for allowing parallel registration/unregistration >> >> of shrinkers. >> >> >> >> Since do_shrink_slab() can reschedule, we cannot protect shrinker_list >> >> using one RCU section. But using atomic_inc()/atomic_dec() for each >> >> do_shrink_slab() call will not impact so much. >> >> >> >> This patch uses polling loop with short sleep for unregister_shrinker() >> >> rather than wait_on_atomic_t(), for we can save reader's cost (plain >> >> atomic_dec() compared to atomic_dec_and_test()), we can expect that >> >> do_shrink_slab() of unregistering shrinker likely returns shortly, and >> >> we can avoid khungtaskd warnings when do_shrink_slab() of unregistering >> >> shrinker unexpectedly took so long. >> >> >> >> Signed-off-by: Tetsuo Handa >> > >> > Before reviewing this patch, can't we solve the problem with more >> > simple way? Like this. >> > >> > Shakeel, What do you think? >> > >> >> Seems simple enough. I will run my test (running fork bomb in one >> memcg and separately time a mount operation) and update if numbers >> differ significantly. > > Thanks. > >> >> > diff --git a/mm/vmscan.c b/mm/vmscan.c >> > index 13d711dd8776..cbb624cb9baa 100644 >> > --- a/mm/vmscan.c >> > +++ b/mm/vmscan.c >> > @@ -498,6 +498,14 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, >> > sc.nid = 0; >> > >> > freed += do_shrink_slab(&sc, shrinker, nr_scanned, nr_eligible); >> > + /* >> > + * bail out if someone want to register a new shrinker to prevent >> > + * long time stall by parallel ongoing shrinking. >> > + */ >> > + if (rwsem_is_contended(&shrinker_rwsem)) { >> > + freed = 1; >> >> freed = freed ?: 1; > > Yub. Thanks Minchan, you can add Reviewed-and-tested-by: Shakeel Butt -- 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: email@kvack.org