Re: [PATCH v4 1/6] fs: enhance and rename shutdown() callback to remove_bdev()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2025/7/9 05:50, Darrick J. Wong 写道:
[...]
Well, I'd also say just go for own fs_holder_ops if it was not for the
awkward "get super from bdev" step. As Christian wrote we've encapsulated
that in fs/super.c and bdev_super_lock() in particular but the calling
conventions for the fs_holder_ops are not very nice (holding
bdev_holder_lock, need to release it before grabbing practically anything
else) so I'd have much greater peace of mind if this didn't spread too
much. Once you call bdev_super_lock() and hold on to sb with s_umount held,
things are much more conventional for the fs land so I'd like if this
step happened before any fs hook got called. So I prefer something like
Qu's proposal of separate sb op for device removal over exporting
bdev_super_lock(). Like:

static void fs_bdev_mark_dead(struct block_device *bdev, bool surprise)
{
         struct super_block *sb;

         sb = bdev_super_lock(bdev, false);
         if (!sb)
                 return;

	if (sb->s_op->remove_bdev) {
		sb->s_op->remove_bdev(sb, bdev, surprise);

The only concern here is, remove_bdev() will handle two different things:

- Mark the target devices as missing and continue working
  Of course that's when the fs can handle it.

- Shutdown
  If the fs can not handle it.

And if the fs has to shutdown, we're missing all the shrinks in the shutdown path.

Thus I'd prefer the remove_bdev() to have a return value, so that we can fallback to shutdown path if the remove_bdev() failed.

This also aligns more towards Brauner's idea that we may want to expose if the fs can handle the removal.

Thanks,
Qu

		return;
	}

It feels odd but I could live with this, particularly since that's the
direction that brauner is laying down. :)

Do we still need to super_unlock_shared here?

--D


	if (!surprise)
		sync_filesystem(sb);
	shrink_dcache_sb(sb);
	evict_inodes(sb);
	if (sb->s_op->shutdown)
		sb->s_op->shutdown(sb);

	super_unlock_shared(sb);
}

As an aside:
'twould be nice if we could lift the *FS_IOC_SHUTDOWN dispatch out of
everyone's ioctl functions into the VFS, and then move the "I am dead"
state into super_block so that you could actually shut down any
filesystem, not just the seven that currently implement it.

Yes, I should find time to revive that patch series... It was not *that*
hard to do.

								Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR







[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux