Re: sb->s_fs_info freeing fixes

Christian Brauner
2023-08-31T10:20:44Z

On Thu, Aug 31, 2023 at 07:31:53AM +0200, Christoph Hellwig wrote:
> sb->s_fs_info should only be freed after the superblock has been marked
> inactive in generic_shutdown_super, which means either in ->put_super or
> in ->kill_sb after generic_shutdown_super has returned. Fix the
> instances where that is not the case.

Funny, I had looked at all those filesystems a while back to
double-check that things are sane and that's why I didn't change them.

> arch/s390/hypfs/inode.c | 3 +--

get_tree_single() -> single instance
=> doesn't match on sb->s_fs_info

> fs/devpts/inode.c | 2 +-

get_tree_nodev() -> each mount is a new instance
=> We don't match on sb->s_fs_info

> fs/ramfs/inode.c | 2 +-

get_tree_nodev() -> each mount is a new instance
=> We don't match on sb->s_fs_info

> security/selinux/selinuxfs.c | 5 +----

get_tree_single() -> single instance
=> doesn't match on sb->s_fs_info

Al roughly said the same thing afaict.

I still think that there's no need to deviate from the basic logic:

(1) call generic kill_*() helper
(2) wipe sb->s_fs_info

So I think that's a cleanup we should do. Just change the rationale to
say that this deviation is pointless and just means the reader of the
code has to sanity check against the superblock helper that's used.

This btw, has confused me before too and I basically had a version of
the exchange we're now having: