Date: Tue, 19 Apr 2016 09:06:01 +1000
From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: Florian Margaine <florian@margaine.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Jeff Layton <jlayton@poochiereds.net>,
        "J. Bruce Fields" <bfields@fieldses.org>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fs: add the FIGETFROZEN ioctl call
Message-ID: <20160418230601.GA18496@dastard>
References: <CABRjmp_FK8PYyAhBjAqE3Er8gfPo+EkJA5+yzVg7SdBYEwjJeA@mail.gmail.com>
 <20160415021737.GZ567@dastard>
 <5714FB36.6020704@sandeen.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5714FB36.6020704@sandeen.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2334
Lines: 66

On Mon, Apr 18, 2016 at 11:20:22AM -0400, Eric Sandeen wrote:
> 
> 
> On 4/14/16 10:17 PM, Dave Chinner wrote:
> > On Thu, Apr 14, 2016 at 09:57:07AM +0200, Florian Margaine wrote:
> >> This lets userland get the filesystem freezing status, aka whether the
> >> filesystem is frozen or not. This is so that an application can know if
> >> it should freeze the filesystem or if it isn't necessary when taking a
> >> snapshot.
> > 
> > freezing nests, so there is no reason for avoiding a freeze when
> > doing a snapshot. 
> 
> Sadly, no:
> 
> # xfs_freeze -f /mnt/test
> # xfs_freeze -f /mnt/test
> xfs_freeze: cannot freeze filesystem at /mnt/test: Device or resource busy
> 
> It used to, but it was broken^Wchanged quite some time ago.

Ugh. Block device freeze nesting still works (i.e. freeze_bdev, as
snapshots from DM would use), but I didn't realise (or had
forgetten) that superblock level freeze nesting had been removed...

> > Indeed, if you don't wrap freeze/thaw around a
> > snapshot, then if the fs is thawed while the snapshot is in progress
> > then you are going to get a corrupt snapshot....
> 
> Yep.
> 
> IMHO what really needs to happen is to fix freeze to allow nesting
> again.

Probably. I quick dig shows nesting was intentionally broken more
than 5 years ago in making the freeze ioctl work on btrfs. 

commit 18e9e5104fcd9a973ffe3eed3816c87f2a1b6cd2
Author: Josef Bacik <josef@redhat.com>
Date:   Tue Mar 23 10:34:56 2010 -0400

    Introduce freeze_super and thaw_super for the fsfreeze ioctl
.....
    The only new gotcha is multiple calls to the fsfreeze ioctl will return EBUSY if
    the fs is already frozen.  I thought this was a better solution than adding a
    freeze counter to the super_block, but if everybody hates this idea I'm open to
    suggestions.  Thanks,
....

Not sure many people noticed that at the time....

> A way to query freeze state might be nice, I think, but yeah, it's
> racy, so you can't depend on it - but it might be useful in the "huh,
> IO is failing, what's going on?  Oh, it's frozen, ok" scenario...

So maybe we should just add the frozen state to /proc/self/mountinfo
or something similar, then people who think it matters can shoot
themselves in the foot all they want without us needing to care
about it.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com