Subject: Re: [PATCH] fs: add the FIGETFROZEN ioctl call
To: Dave Chinner <david@fromorbit.com>,
        Florian Margaine <florian@margaine.com>
References: <CABRjmp_FK8PYyAhBjAqE3Er8gfPo+EkJA5+yzVg7SdBYEwjJeA@mail.gmail.com>
 <20160415021737.GZ567@dastard>
Cc: 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
From: Eric Sandeen <sandeen@sandeen.net>
Message-ID: <5714FB36.6020704@sandeen.net>
Date: Mon, 18 Apr 2016 11:20:22 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:38.0)
 Gecko/20100101 Thunderbird/38.7.2
MIME-Version: 1.0
In-Reply-To: <20160415021737.GZ567@dastard>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1858
Lines: 60


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.

> 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.

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...

But if you want to be sure your snapshot is OK even while others are
running concurrent snapshots, we need nested freezes to work again.

-Eric

> And, besides, polling for frozenness from userspace is inherently
> racy - by the time the syscall returns, the information may be
> incorrect, so you can't rely on it for decision making purposes in
> userspace.
> 
>> +static int ioctl_fsgetfrozen(struct file *filp)
>> +{
>> + struct super_block *sb = file_inode(filp)->i_sb;
>> +
>> + if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + return sb->s_writers.frozen;
> 
> This makes the internal freeze implementation states part of the
> userspace ABI.  This needs an API that is separate from the internal
> implementation...
> 
> Cheers,
> 
> Dave.
>