linux-ext4 - Re: [PATCH v4 1/6] fs: enhance and rename shutdown() callback to remove

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <31d4c67f-160b-456d-a47b-869ddc5be6d0@gmx.com>
Date: Tue, 8 Jul 2025 15:11:43 +0930
From: Qu Wenruo <quwenruo.btrfs@....com>
To: Dave Chinner <david@...morbit.com>
Cc: "Darrick J. Wong" <djwong@...nel.org>, Qu Wenruo <wqu@...e.com>,
 linux-btrfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 viro@...iv.linux.org.uk, brauner@...nel.org, jack@...e.cz,
 linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
 ntfs3@...ts.linux.dev, linux-xfs@...r.kernel.org
Subject: Re: [PATCH v4 1/6] fs: enhance and rename shutdown() callback to
 remove_bdev()

在 2025/7/8 14:35, Dave Chinner 写道:
[...]
>>> Not really worthy if we only want a single different behavior.
> 
> This is the *3rd* different behaviour for ->mark_dead. We
> have the generic behaviour, the bcachefs behaviour, and now the
> btrfs behaviour (whatever that may be).

Then why not merging the btrfs/bcachefs callback into one generic 
callback? Other than introducing 3 different bdev_holder_ops?

This looks exactly the opposite what VFS is trying to do.

Thanks,
Qu

> 
>>> Thus I strongly prefer to do with the existing fs_holder_ops, no matter
>>> if it's using/renaming the shutdown() callback, or a new callback.
>>
>> Previously Christoph is against a new ->remove_bdev() callback, as it is
>> conflicting with the existing ->shutdown().
>>
>> So what about a new ->handle_bdev_remove() callback, that we do something
>> like this inside fs_bdev_mark_dead():
>>
>> {
>> 	bdev_super_lock();
>> 	if (!surprise)
>> 		sync_filesystem();
>>
>> 	if (s_op->handle_bdev_remove) {
>> 		ret = s_op->handle_bdev_remove();
>> 		if (!ret) {
>> 			super_unlock_shared();
>> 			return;
>> 		}
>> 	}
>> 	shrink_dcache_sb();
>> 	evict_inodes();
>> 	if (s_op->shutdown)
>> 		s_op->shutdown();
>> }
>>
>> So that the new ->handle_bdev_remove() is not conflicting with
>> ->shutdown() but an optional one.
>>
>> And if the fs can not handle the removal, just let
>> ->handle_bdev_remove() return an error so that we fallback to the existing
>> shutdown routine.
>>
>> Would this be more acceptable?
> 
> No.
> 
> -Dave.