All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Dave Chinner <david@fromorbit.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] change thaw_super() to re-acquire s_writers.lock_map
Date: Tue, 21 Jul 2015 10:48:13 +0200	[thread overview]
Message-ID: <20150721084813.GD6533@quack.suse.cz> (raw)
In-Reply-To: <20150720170109.GA3909@redhat.com>

On Mon 20-07-15 19:01:09, Oleg Nesterov wrote:
> Change thaw_super() to re-acquire the "write" locks we are going to
> release. This way s_op->unfreeze_fs(sb) is called with these locks
> held as it seen by lockdep, and this matches the reality. This adds
> another trivial helper, sb_freeze_acquire().
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Ah, I was confused for a moment why this doesn't trigger a lockdep warning
because of lock inversion between s_umount and freeze protection but you
use trylock in sb_freeze_acquire(). So things are fine.

You can add:

Reviewed-by: Jan Kara <jack@suse.com>

								Honza

> ---
>  fs/super.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index e7ea9f1..b4db3ee 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1263,6 +1263,14 @@ static void sb_freeze_release(struct super_block *sb)
>  		rwsem_release(sb->s_writers.lock_map + level, 1, _THIS_IP_);
>  }
>  
> +static void sb_freeze_acquire(struct super_block *sb)
> +{
> +	int level;
> +
> +	for (level = 0; level < SB_FREEZE_LEVELS; ++level)
> +		rwsem_acquire(sb->s_writers.lock_map + level, 0, 1, _THIS_IP_);
> +}
> +
>  /**
>   * freeze_super - lock the filesystem and force it into a consistent state
>   * @sb: the super to lock
> @@ -1386,16 +1394,20 @@ int thaw_super(struct super_block *sb)
>  	if (sb->s_flags & MS_RDONLY)
>  		goto out;
>  
> +	sb_freeze_acquire(sb);
> +
>  	if (sb->s_op->unfreeze_fs) {
>  		error = sb->s_op->unfreeze_fs(sb);
>  		if (error) {
>  			printk(KERN_ERR
>  				"VFS:Filesystem thaw failed\n");
> +			sb_freeze_release(sb);
>  			up_write(&sb->s_umount);
>  			return error;
>  		}
>  	}
>  
> +	sb_freeze_release(sb);
>  out:
>  	sb->s_writers.frozen = SB_UNFROZEN;
>  	smp_wmb();
> -- 
> 1.5.5.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

      reply	other threads:[~2015-07-21  8:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-20 17:00 [PATCH 0/4] sb_write: lockdep fixes/cleanups Oleg Nesterov
2015-07-20 17:01 ` [PATCH 1/4] introduce __sb_{acquire,release}_write() helpers Oleg Nesterov
2015-07-21  8:23   ` Jan Kara
2015-07-20 17:01 ` [PATCH 2/4] fix the broken lockdep logic in __sb_start_write() Oleg Nesterov
2015-07-21  8:38   ` Jan Kara
2015-07-22 21:13     ` Oleg Nesterov
2015-07-20 17:01 ` [PATCH 3/4] move rwsem_release() from sb_wait_write() to freeze_super() Oleg Nesterov
2015-07-21  8:40   ` Jan Kara
2015-07-20 17:01 ` [PATCH 4/4] change thaw_super() to re-acquire s_writers.lock_map Oleg Nesterov
2015-07-21  8:48   ` Jan Kara [this message]

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=20150721084813.GD6533@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@fromorbit.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /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.