On Thu 26-10-23 14:07:38, Christian Brauner wrote:
> On Thu, Oct 26, 2023 at 12:35:03PM +0200, Jan Kara wrote:
> > This is about those seemingly spurious "device busy" errors when the
> > superblock hasn't closed its devices yet, isn't it? But we now remove
>
> Yes, because we tie superblock and block device neatly together.
>
> > superblock from s_instances list in kill_super_notify() and until that
> > moment SB_DYING is protecting us from racing mounts. So is there some other
> > problem?
>
> No, there isn't a problem at all. It's all working fine but it was
> initially a little annoying as we had to update filesystems to ensure
> that sb->s_fs_info is kept alive. But it's all fixed.
>
> The possible advantage is that if we drop all block devices under
> s_umount then we can remove the superblock from fs_type->s_instances in
> the old location again. I'm not convinced it's worth it but it's a
> possible simplification. I'm not even arguing it's objectively better I
> think it's a matter of taste in the end.
Yes. But dropping the main bdev under s_umount is not easy to do with the
current callback structure. We have kill_block_super() calling into
generic_shutdown_super(). Now logically generic_shutdown_super() wants to
teardown bdi (for which it needs to drop s_umount) but bdev is closed only
in kill_block_super() because that's the sb-on-bdev specific call.
Previously we got away with this without spurious EBUSY errors because the
bdev holder was the filesystem type and not the superblock. But after
changing the holder it isn't fine anymore and we have to play these games
with s_instances and SB_DYING.
What we could probably do is to have something like:
if (sb->s_bdev) {
blkdev_put(sb->s_bdev);
sb->s_bdev = NULL;
}
if (sb->s_mtd) {
put_mtd_device(sb->s_mtd);
sb->s_mtd = NULL;
}
in generic_shutdown_super() and then remove sb from s_instances, drop
s_umount and cleanup bdi.
Then we can get rid of SB_DYING, kill_super_notify() and stuff but based on
your taste it could be also viewed as kind of layering violation so I'm not
100% convinced this is definitely a way to go.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
