This is generated against: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs-6.18.inode.refcount.preliminaries The first patch in the series is ready to use(tm) and was sent separately here: https://lore.kernel.org/linux-fsdevel/20250909082613.1296550-1-mjguzik@xxxxxxxxx/T/#u It is included in this posting as the newly patched routine has to get patched further due to the i_state accessor thing. Having it here should make it handier to test for interested. Compared to v2 this drops I_WILL_FREE removal for the time being and converts the entirety of the kernel. Patch rundown: fs: hide ->i_state handling behind accessors Provides the new helpers and converts general kernel (fs/*.c and others, but no specific filesystems). ->i_state temporarily remains accessible with plain loads/stores. convert all filesystems to use the new ->i_state accessors This is a squashed patch to avoid sending a 50-mail churn patchset at this stage, included for reference for interested how things are going to look like. This will be one patch per fs in the actual submission. Conversion generated with coccinelle + manual fixups. I got a kernel with ext4 running, I have not tested xfs or btrfs yet. type switch This hides ->i_state behind a struct, so things nicely fail to compile if someone open-codes a plain access. Rationale: The handling in the stock kernel is highly error prone and with 0 assert coverage. The ->i_state field is legally accessible for reads without holding the inode spinlock. This opens up potential for compiler mischief in code doing things like: state = inode->i_state; if (state & I_FOO) .... else if (state & I_BAR) .... Other code may be expecting the value to be stable (only guaranteed with the spinlock held) and unknowingly being called without that guarantee. Some places modifying or reading the field use WRITE_ONCE/READ_ONCE respectively, but others don't. Modifying flags requires the lock, but right now it is never checked if such operations are done while it is being held. Even ignoring that aspect, all current state modifications lack any validation whether a given change is legal to do. For example there may be things which are illegal to do once I_FREEING is set, but the current code fails to check for it in any capacity. With the assumption the general intent is to tighten up the interfaces, I think this is a step forward. Note the new routines can assert on ->i_state changes based on other parts of the inode (trivial example: the routine can validate nobody sets I_FREEING if there is a refcount > 0). The proposed routines still remain kind of lax in what they do, notably there are _unchecked variants to bypass any validation (used in special cases and by XFS). To be tightened up over time. As sample usage I'm going to quote part of iput_final() with the infamous I_WILL_FREE flag: if (!drop) { inode_state_add(inode, I_WILL_FREE); spin_unlock(&inode->i_lock); write_inode_now(inode, 1); spin_lock(&inode->i_lock); inode_state_del(inode, I_WILL_FREE); WARN_ON(inode_state_read(inode) & I_NEW); } inode_state_add(inode, I_FREEING); Lockless loads are a little bit verbose though: VFS_BUG_ON_INODE(inode_state_read_unstable(inode) & I_CLEAR, inode); I'm not going to argue about naming, it can be trivially changed with sed on the patchset. I do claim whacking open-coded access to ->i_state is a mandatory part of whipping VFS into a better shape in terms of maintainability. I'm not going to repeat myself how and why, a description can be found in my responses to the i_obj_count thread: https://lore.kernel.org/linux-fsdevel/dpi5ey667awq63mgxrzu6qdfpeinrmeapgbllqidcdjayanz2p@kp3alvfskssp/ https://lore.kernel.org/linux-fsdevel/frffkyu6vac5v7qt5ee36xmg6hrwwdks7mnn2k7krdqecn56mc@3kwx24xtmane/ If this looks good I'll write proper commit messages and whatnot, do some more testing and properly resend. Mateusz Guzik (4): fs: expand dump_inode() fs: hide ->i_state handling behind accessors convert all filesystems to use the new ->i_state accessors type switch block/bdev.c | 4 +- drivers/dax/super.c | 2 +- fs/9p/vfs_inode.c | 2 +- fs/9p/vfs_inode_dotl.c | 2 +- fs/affs/inode.c | 2 +- fs/afs/dynroot.c | 6 +- fs/afs/inode.c | 8 +- fs/bcachefs/fs.c | 7 +- fs/befs/linuxvfs.c | 2 +- fs/bfs/inode.c | 2 +- fs/btrfs/inode.c | 10 +-- fs/buffer.c | 4 +- fs/ceph/cache.c | 2 +- fs/ceph/crypto.c | 4 +- fs/ceph/file.c | 4 +- fs/ceph/inode.c | 28 +++---- fs/coda/cnode.c | 4 +- fs/cramfs/inode.c | 2 +- fs/crypto/keyring.c | 2 +- fs/crypto/keysetup.c | 2 +- fs/dcache.c | 8 +- fs/drop_caches.c | 2 +- fs/ecryptfs/inode.c | 6 +- fs/efs/inode.c | 2 +- fs/erofs/inode.c | 2 +- fs/ext2/inode.c | 2 +- fs/ext4/inode.c | 10 +-- fs/ext4/orphan.c | 4 +- fs/f2fs/data.c | 2 +- fs/f2fs/inode.c | 2 +- fs/f2fs/namei.c | 4 +- fs/f2fs/super.c | 2 +- fs/freevxfs/vxfs_inode.c | 2 +- fs/fs-writeback.c | 123 ++++++++++++++++--------------- fs/fuse/inode.c | 4 +- fs/gfs2/file.c | 2 +- fs/gfs2/glops.c | 2 +- fs/gfs2/inode.c | 4 +- fs/gfs2/ops_fstype.c | 2 +- fs/hfs/btree.c | 2 +- fs/hfs/inode.c | 2 +- fs/hfsplus/super.c | 2 +- fs/hostfs/hostfs_kern.c | 2 +- fs/hpfs/dir.c | 2 +- fs/hpfs/inode.c | 2 +- fs/inode.c | 111 +++++++++++++++------------- fs/isofs/inode.c | 2 +- fs/jffs2/fs.c | 4 +- fs/jfs/file.c | 4 +- fs/jfs/inode.c | 2 +- fs/jfs/jfs_txnmgr.c | 2 +- fs/kernfs/inode.c | 2 +- fs/libfs.c | 6 +- fs/minix/inode.c | 2 +- fs/namei.c | 8 +- fs/netfs/misc.c | 8 +- fs/netfs/read_single.c | 6 +- fs/nfs/inode.c | 2 +- fs/nfs/pnfs.c | 2 +- fs/nfsd/vfs.c | 2 +- fs/nilfs2/cpfile.c | 2 +- fs/nilfs2/dat.c | 2 +- fs/nilfs2/ifile.c | 2 +- fs/nilfs2/inode.c | 10 +-- fs/nilfs2/sufile.c | 2 +- fs/notify/fsnotify.c | 2 +- fs/ntfs3/inode.c | 2 +- fs/ocfs2/dlmglue.c | 2 +- fs/ocfs2/inode.c | 10 +-- fs/omfs/inode.c | 2 +- fs/openpromfs/inode.c | 2 +- fs/orangefs/inode.c | 2 +- fs/orangefs/orangefs-utils.c | 6 +- fs/overlayfs/dir.c | 2 +- fs/overlayfs/inode.c | 6 +- fs/overlayfs/util.c | 10 +-- fs/pipe.c | 2 +- fs/qnx4/inode.c | 2 +- fs/qnx6/inode.c | 2 +- fs/quota/dquot.c | 2 +- fs/romfs/super.c | 2 +- fs/smb/client/cifsfs.c | 2 +- fs/smb/client/inode.c | 14 ++-- fs/squashfs/inode.c | 2 +- fs/sync.c | 2 +- fs/ubifs/file.c | 2 +- fs/ubifs/super.c | 2 +- fs/udf/inode.c | 2 +- fs/ufs/inode.c | 2 +- fs/xfs/scrub/common.c | 2 +- fs/xfs/scrub/inode_repair.c | 2 +- fs/xfs/scrub/parent.c | 2 +- fs/xfs/xfs_bmap_util.c | 2 +- fs/xfs/xfs_health.c | 4 +- fs/xfs/xfs_icache.c | 6 +- fs/xfs/xfs_inode.c | 6 +- fs/xfs/xfs_inode_item.c | 4 +- fs/xfs/xfs_iops.c | 2 +- fs/xfs/xfs_reflink.h | 2 +- fs/zonefs/super.c | 4 +- include/linux/backing-dev.h | 5 +- include/linux/fs.h | 59 ++++++++++++++- include/linux/writeback.h | 4 +- include/trace/events/writeback.h | 8 +- mm/backing-dev.c | 2 +- security/landlock/fs.c | 2 +- 106 files changed, 368 insertions(+), 309 deletions(-) -- 2.43.0
