All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Jeff Layton <jeff.layton@primarydata.com>
Cc: linux-fsdevel@vger.kernel.org, trond.myklebust@primarydata.com,
	tao.peng@primarydata.com
Subject: Re: [PATCH] vfs: remove unneeded hlist_unhashed check from get_active_super
Date: Tue, 14 Oct 2014 20:13:26 +0100	[thread overview]
Message-ID: <20141014191326.GZ7996@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20141014071134.24a8ff68@tlielax.poochiereds.net>

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.

  reply	other threads:[~2014-10-14 19:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-14 10:36 [PATCH] vfs: remove unneeded hlist_unhashed check from get_active_super Jeff Layton
2014-10-14 11:11 ` Jeff Layton
2014-10-14 19:13   ` Al Viro [this message]
2014-10-14 19:19     ` Jeff Layton
2014-10-14 23:05       ` Al Viro

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=20141014191326.GZ7996@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=jeff.layton@primarydata.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tao.peng@primarydata.com \
    --cc=trond.myklebust@primarydata.com \
    /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: link
Be 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.