Re: [RFC v2] fs: add prepare_freeze/prepare_thaw fs hooks

[Dave Chinner <david@fromorbit.com>]

On Wed, Sep 24, 2014 at 02:01:58PM -0500, Benjamin Marzinski wrote:
> Currently, freezing a filesystem involves calling freeze_super, which locks
> sb->s_umount and then calls the fs-specific freeze_fs hook. This makes it
> hard for gfs2 (and potentially other cluster filesystems) to use the vfs
> freezing code to do freezes on all the cluster nodes.
> 
> In order to communicate that a freeze has been requested, and to make sure
> that only one node is trying to freeze at a time, gfs2 uses a glock
> (sd_freeze_gl). The problem is that there is no hook for gfs2 to acquire
> this lock before calling freeze_super. This means that two nodes can
> attempt to freeze the filesystem by both calling freeze_super, acquiring
> the sb->s_umount lock, and then attempting to grab the cluster glock
> sd_freeze_gl. Only one will succeed, and the other will be stuck in
> freeze_super, making it impossible to finish freezing the node.
> 
> To solve this problem, this patch adds the prepare_freeze prepare_thaw
> hooks.  If a filesystem implements these hooks, they are called instead of
> freeze_super and thaw_super. This means that every filesystem that
> implements prepare_freeze/thaw must call freeze/thaw_super within that
> function to make use of the vfs freezing code.

Why instead? The filesystem still have to call
freeze_super/thaw_super() after "prepare".

> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  fs/block_dev.c     | 10 ++++++++--
>  fs/ioctl.c         |  6 +++++-
>  include/linux/fs.h |  2 ++
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 6d72746..f931412 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -245,7 +245,10 @@ struct super_block *freeze_bdev(struct block_device *bdev)
>  	sb = get_active_super(bdev);
>  	if (!sb)
>  		goto out;
> -	error = freeze_super(sb);
> +	if (sb->s_op->prepare_freeze)
> +		error = sb->s_op->prepare_freeze(sb);
> +	else
> +		error = freeze_super(sb);

I was proposing this callout to be like this:

	if (sb->s_op->prepare_freeze)
		error = sb->s_op->prepare_freeze(sb);
	if (!error)
		error = freeze_super(sb);
	if (error) {
		....

i.e. prepare() does all the filesystem specific preparation,
everything else goes through the normal freeze code.  The lack of
gfs2 specific patches showing how gfs2 is going to use this
interface is not helping here...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com