On Tue, 14 Oct 2014 06:36:55 -0400
Jeff Layton <jlayton@xxxxxxxxxxxxxxx> wrote:
> As best I can tell, a superblock is only ever removed from the fs_supers
> list after the s_active counter goes to zero. Thus, the hlist_unhashed
> check here is redundant, since the atomic_inc_not_zero check in
> grab_super will catch that case anyway.
>
> Cc: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> Cc: Peng Tao <tao.peng@xxxxxxxxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
> ---
> fs/super.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index eae088f6aaae..7ff277547c1e 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -320,9 +320,7 @@ EXPORT_SYMBOL(deactivate_super);
> * and want to turn it into a full-blown active reference. grab_super()
> * is called with sb_lock held and drops it. Returns 1 in case of
> * success, 0 if we had failed (superblock contents was already dead or
> - * dying when grab_super() had been called). Note that this is only
> - * called for superblocks not in rundown mode (== ones still on ->fs_supers
> - * of their type), so increment of ->s_count is OK here.
> + * dying when grab_super() had been called).
> */
> static int grab_super(struct super_block *s) __releases(sb_lock)
> {
> @@ -641,8 +639,6 @@ struct super_block *get_active_super(struct block_device *bdev)
> restart:
> spin_lock(&sb_lock);
> list_for_each_entry(sb, &super_blocks, s_list) {
> - if (hlist_unhashed(&sb->s_instances))
> - continue;
> if (sb->s_bdev == bdev) {
> if (!grab_super(sb))
> goto restart;
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?
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
