Re: [PATCH v4 5/6] btrfs: implement shutdown ioctl

[Qu Wenruo <wqu@suse.com>]

在 2025/7/5 23:52, David Sterba 写道:
> On Fri, Jul 04, 2025 at 10:12:33AM +0930, Qu Wenruo wrote:
>> The shutdown ioctl should follow the XFS one, which use magic number 'X',
>> and ioctl number 125, with a u32 as flags.
>>
>> For now btrfs don't distinguish DEFAULT and LOGFLUSH flags (just like
>> f2fs), both will freeze the fs first (implies committing the current
>> transaction), setting the SHUTDOWN flag and finally thaw the fs.
>>
>> For NOLOGFLUSH flag, the freeze/thaw part is skipped thus the current
>> transaction is aborted.
>>
>> The new shutdown ioctl is hidden behind experimental features for more
>> testing.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/ioctl.c           | 40 ++++++++++++++++++++++++++++++++++++++
>>   include/uapi/linux/btrfs.h |  9 +++++++++
>>   2 files changed, 49 insertions(+)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 2f3b7be13bea..94eb7a8499db 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -5194,6 +5194,36 @@ static int btrfs_ioctl_subvol_sync(struct btrfs_fs_info *fs_info, void __user *a
>>   	return 0;
>>   }
>>   
>> +#ifdef CONFIG_BTRFS_EXPERIMENTAL
>> +static int btrfs_emergency_shutdown(struct btrfs_fs_info *fs_info, u32 flags)
>> +{
>> +	int ret = 0;
>> +
>> +	if (flags >= BTRFS_SHUTDOWN_FLAGS_LAST)
>> +		return -EINVAL;
>> +
>> +	if (btrfs_is_shutdown(fs_info))
>> +		return 0;
>> +
>> +	switch (flags) {
>> +	case BTRFS_SHUTDOWN_FLAGS_LOGFLUSH:
>> +	case BTRFS_SHUTDOWN_FLAGS_DEFAULT:
>> +		ret = freeze_super(fs_info->sb, FREEZE_HOLDER_KERNEL, NULL);
> 
> Recently I've looked at scrub blocking filesystem freezing and it does
> not work because it blocks on the semaphore taken in mnt_want_write,
> also taken in freeze_super().
> 
> I have an idea for fix, basically pause scrub, undo mnt_want_write
> and then call freeze_super. So we'll need that too for shutdown. Once
> implemented the fixup would be to use btrfs_freeze_super callback here.

It may not be that simple.

freeze_super() itself is doing extra works related to the 
stage/freeze_owner/etc.

I'm not sure if it's a good idea to completely skip that part.

I'd prefer scrub to check the frozen stage, and if it's already in any 
FREEZE stages, exit early.

Thanks,
Qu

> 
>> +		if (ret)
>> +			return ret;
>> +		btrfs_force_shutdown(fs_info);
>> +		ret = thaw_super(fs_info->sb, FREEZE_HOLDER_KERNEL, NULL);
>> +		if (ret)
>> +			return ret;
>> +		break;
>> +	case BTRFS_SHUTDOWN_FLAGS_NOLOGFLUSH:
>> +		btrfs_force_shutdown(fs_info);
>> +		break;
>> +	}
>> +	return ret;
>> +}
>> +#endif
>> +
>>   long btrfs_ioctl(struct file *file, unsigned int
>>   		cmd, unsigned long arg)
>>   {
> 
>> --- a/include/uapi/linux/btrfs.h
>> +++ b/include/uapi/linux/btrfs.h
>> @@ -1096,6 +1096,12 @@ enum btrfs_err_code {
>>   	BTRFS_ERROR_DEV_RAID1C4_MIN_NOT_MET,
>>   };
>>   
>> +/* Flags for IOC_SHUTDOWN, should match XFS' flags. */
>> +#define BTRFS_SHUTDOWN_FLAGS_DEFAULT	0x0
>> +#define BTRFS_SHUTDOWN_FLAGS_LOGFLUSH	0x1
>> +#define BTRFS_SHUTDOWN_FLAGS_NOLOGFLUSH	0x2
>> +#define BTRFS_SHUTDOWN_FLAGS_LAST	0x3
>> +
>>   #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
>>   				   struct btrfs_ioctl_vol_args)
>>   #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
>> @@ -1217,6 +1223,9 @@ enum btrfs_err_code {
>>   #define BTRFS_IOC_SUBVOL_SYNC_WAIT _IOW(BTRFS_IOCTL_MAGIC, 65, \
>>   					struct btrfs_ioctl_subvol_wait)
>>   
>> +/* Shutdown ioctl should follow XFS's interfaces, thus not using btrfs magic. */
>> +#define BTRFS_IOC_SHUTDOWN	_IOR('X', 125, __u32)
> 
> In XFS it's
> 
> #define XFS_IOC_GOINGDOWN            _IOR ('X', 125, uint32_t)
> 
> It's right to use the same definition and ioctl value as this will
> be a generic ioctl eventually, with 3 users at least. I like the name
> SHUTDOWN better, ext4 also uses that.