From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH] vfs: remove unneeded hlist_unhashed check from get_active_super Date: Tue, 14 Oct 2014 20:13:26 +0100 Message-ID: <20141014191326.GZ7996@ZenIV.linux.org.uk> References: <1413283015-2609-1-git-send-email-jlayton@primarydata.com> <20141014071134.24a8ff68@tlielax.poochiereds.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, trond.myklebust@primarydata.com, tao.peng@primarydata.com To: Jeff Layton Return-path: Received: from zeniv.linux.org.uk ([195.92.253.2]:40252 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754278AbaJNTN2 (ORCPT ); Tue, 14 Oct 2014 15:13:28 -0400 Content-Disposition: inline In-Reply-To: <20141014071134.24a8ff68@tlielax.poochiereds.net> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Oct 14, 2014 at 07:11:34AM -0400, Jeff Layton wrote: > Ugh. Now that I've sent the patch, I think this is bogus, so I'll go > ahead and NAK it myself... > > The problem is not the s_active counter, but the fact that grab_super > bumps the s_count unconditionally. So, this code seems to be using the > hlist_unhashed check to verify that it's ok to bump the s_count there. > > That said, I don't quite get why it uses that as the check -- would it > not be more straightforward to simply have grab_super and other callers > check for a 0->1 transition of the s_count? Because there might be *other* holders of ->s_count waiting to run down. Basically, you reintroduce a nasty livelock that way. Once the superblock is past the shutdown, ->s_count can go only down. Somebody might be holding it while waiting for ->s_umount to be acquired, but all such processes will detect that it's dead as soon as they get ->s_umount, drop it and drop ->s_count. Allowing e.g. grab_super() to _increment_ ->s_count and try to get ->s_umount at that stage would reopen a livelock we used to have.