From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752944AbdKWPoJ (ORCPT ); Thu, 23 Nov 2017 10:44:09 -0500 Received: from zeniv.linux.org.uk ([195.92.253.2]:41804 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752611AbdKWPoI (ORCPT ); Thu, 23 Nov 2017 10:44:08 -0500 Date: Thu, 23 Nov 2017 15:44:04 +0000 From: Al Viro To: Tetsuo Handa Cc: mhocko@kernel.org, david@fromorbit.com, jack@suse.cz, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org Subject: Re: [PATCH v2] fs: handle shrinker registration failure in sget_userns Message-ID: <20171123154404.GD21978@ZenIV.linux.org.uk> References: <20171123115247.30685-1-mhocko@kernel.org> <20171123143537.ztpxpk3sjbpo72rf@dhcp22.suse.cz> <20171123145540.GB21978@ZenIV.linux.org.uk> <201711240004.GGE82352.MVFtFLHOOOFSQJ@I-love.SAKURA.ne.jp> <20171123152819.GC21978@ZenIV.linux.org.uk> <201711240035.AGF64509.HLOFFOQSMFOJtV@I-love.SAKURA.ne.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201711240035.AGF64509.HLOFFOQSMFOJtV@I-love.SAKURA.ne.jp> User-Agent: Mutt/1.9.0 (2017-09-02) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Nov 24, 2017 at 12:35:29AM +0900, Tetsuo Handa wrote: > Al Viro wrote: > > On Fri, Nov 24, 2017 at 12:04:23AM +0900, Tetsuo Handa wrote: > > > Al Viro wrote: > > > > On Thu, Nov 23, 2017 at 03:35:37PM +0100, Michal Hocko wrote: > > > > > Hopefully less screwed version. But as I've said I am not really > > > > > familiar with the code and do not feel competent to change it so please > > > > > be very careful here. I've moved the shrinker registration to > > > > > alloc_super which turned out to be simpler. > > > > > > > > I don't get it. Why the hell do we need all that PITA in the first place? > > > > Just make sget_userns() end with > > > > if (unlikely(regsiter_shrinker(&s->s_shrink) != 0)) { > > > > deactivate_locked_super(s); > > > > s = ERR_PTR(-ENOMEM); > > > > } > > > > return s; > > > > and be done with that. All there is to it... > > > > > > > > > > Doesn't deactivate_locked_super() call unregister_shrinker() ? > > > > And? unregister_shrinker() will do list_del() on empty list_head > > and kfree(NULL); where's the problem with that? > > > The problem is that calling unregister_shrinker() without successful > register_shrinker() causes crash due to s_shrink.list.{prev,next} == NULL. *shrug* shrinker->nr_deferred = kzalloc(size, GFP_KERNEL); if (!shrinker->nr_deferred) { /* make sure it's in consistent state */ INIT_LIST_HEAD(&shrinker->list); return -ENOMEM; }