Re: [RFC] fs: move freeze/unfreeze_fs hooks before freeze/thaw_super

[Dave Chinner <david@fromorbit.com>]

[please don't top post]

On Tue, Sep 23, 2014 at 08:24:23AM -0500, Benjamin Marzinski wrote:
> On Tue, Sep 23, 2014 at 11:38:59PM +1000, Dave Chinner wrote:
> > On Mon, Sep 22, 2014 at 09:06:41PM -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 pushes the freeze/unfreeze_fs hooks to
> > > before freeze/thaw_super, and makes freeze/thaw_super take a callback
> > > function to execute any fs specific code that needs to be done while
> > > s_umount is held. This also means that every filesystem that implements
> > > freeze/unfreeze_fs must call freeze/thaw_super from that function.
> > 
> > Wouldn't just adding a ->prepare_freeze/->prepare_thaw method pair
> > and adding gfs2 cluster locking to those new methods be better?
> > That way no other filesystem code needs to change at all...
> 
> That way would certainly require fewer changes. But it seemed to me like
> adding a pair of hooks that only gfs2 needed would have less support. If
> people would prefer a patch like that, I can easily change it.

It's much better than changing the meaning and usage of an existing
method and silently breaking every out-of-tree module that uses the
freezefs/unfreezefs methods. Adding new methods is simple, these new
methods have very little impact on the structure and logic of the
code, and it can be ignored by the majority of filesystems. Seems
like the right thing to do to me...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com