From: Mateusz Guzik <mjguzik@gmail.com>
To: brauner@kernel.org
Cc: viro@zeniv.linux.org.uk, jack@suse.cz,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
josef@toxicpanda.com, kernel-team@fb.com, amir73il@gmail.com,
linux-btrfs@vger.kernel.org, linux-ext4@vger.kernel.org,
linux-xfs@vger.kernel.org, ceph-devel@vger.kernel.org,
linux-unionfs@vger.kernel.org, Mateusz Guzik <mjguzik@gmail.com>
Subject: [PATCH v4 00/12] hide ->i_state behind accessors
Date: Tue, 16 Sep 2025 15:58:48 +0200 [thread overview]
Message-ID: <20250916135900.2170346-1-mjguzik@gmail.com> (raw)
This is generated against:
https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs-6.18.inode.refcount.preliminaries
First commit message quoted verbatim with rationable + API:
[quote]
Open-coded accesses prevent asserting they are done correctly. One
obvious aspect is locking, but significantly more can checked. For
example it can be detected when the code is clearing flags which are
already missing, or is setting flags when it is illegal (e.g., I_FREEING
when ->i_count > 0).
In order to keep things manageable this patchset merely gets the thing
off the ground with only lockdep checks baked in.
Current consumers can be trivially converted.
Suppose flags I_A and I_B are to be handled, then if ->i_lock is held:
state = inode->i_state => state = inode_state_read(inode)
inode->i_state |= (I_A | I_B) => inode_state_add(inode, I_A | I_B)
inode->i_state &= ~(I_A | I_B) => inode_state_del(inode, I_A | I_B)
inode->i_state = I_A | I_B => inode_state_set(inode, I_A | I_B)
If ->i_lock is not held or only held conditionally, add "_once"
suffix for the read routine or "_raw" for the rest:
state = inode->i_state => state = inode_state_read_once(inode)
inode->i_state |= (I_A | I_B) => inode_state_add_raw(inode, I_A | I_B)
inode->i_state &= ~(I_A | I_B) => inode_state_del_raw(inode, I_A | I_B)
inode->i_state = I_A | I_B => inode_state_set_raw(inode, I_A | I_B)
The "_once" vs "_raw" discrepancy stems from the read variant differing
by READ_ONCE as opposed to just lockdep checks.
[/quote]
A series with one patch per subsystem/filesystem is quite big (over 50
largely trivial mails) and that's probably not warranted. Instead, core
kernel was handled in one commit and only file systems with changes
which should be looked at got split (all the rest is one combined commit).
This all mostly mechanical churn and that alone should not be
objectional. If someone does not like the API, they should raise it
here.
per-fs postings are there in case something is correctly marked as
unlocked access.
Note that in the worst case should a mistake be made here, it either
will fail to spot ->i_lock not being held (which is equivalent to the
stock state) OR it will generate a lockdep splat. While not pretty, it
is loud and readily fixable. Otherwise this patchset is a NOP.
Testing was limited:
kernel with CONFIG_DEBUG_VFS + lockdep was booted with ext4, survived
kernel builds and whatnot. xfs and btrfs filesystems were mounted had
files linked and unlinked on them.
I very much will need someone with more resources to give this a
beating. I tried to err on the side of expecting the caller does *need*
->i_lock and it is possible something is using inode_state_read()
instead of inode_state_read_once() as a result. If so, there will be a
lockdep splat though.
Coccinelle was used to do the conversion, with all changes audited +
some manual fixups (more eyes welcome):
@@
expression inode, flags;
@@
- inode->i_state & flags
+ inode_state_read(inode) & flags
@@
expression inode, flags;
@@
- inode->i_state &= ~flags
+ inode_state_del(inode, flags)
@@
expression inode, flags;
@@
- inode->i_state |= flags
+ inode_state_add(inode, flags)
@@
expression inode, flags;
@@
- inode->i_state = flags
+ inode_state_set_raw(inode, flags)
Patch breakdown:
fs: provide accessors for ->i_state
This only adds the routines, nothing is using them and overall it's a
NOP.
fs: use ->i_state accessors in core kernel
Converts the entirety of the kernel modulo specific file systems.
fs: mechanically convert most filesystems to use ->i_state accessors
This includes all trivial changes (mostly when the filesystem just
checks for I_NEW after getting the inode from the hash).
btrfs: use the new ->i_state accessors
netfs: use the new ->i_state accessors
nilfs2: use the new ->i_state accessors
xfs: use the new ->i_state accessors
ext4: use the new ->i_state accessors
f2fs: use the new ->i_state accessors
ceph: use the new ->i_state accessors
overlayfs: use the new ->i_state accessors
Per-fs split if there was more work in the area just to sanity check by
interested parties.
fs: make plain ->i_state access fail to compile
This hides ->i_state behind a struct, so things nicely fail to compile
if someone open-codes plain access.
v3:
- rename accessors (s/unchecked/raw; s/unstable/once/)
- rebase
- provide actual commit messages
- per fs patches as I deemed applicable
Mateusz Guzik (12):
fs: provide accessors for ->i_state
fs: use ->i_state accessors in core kernel
fs: mechanically convert most filesystems to use ->i_state accessors
btrfs: use the new ->i_state accessors
netfs: use the new ->i_state accessors
nilfs2: use the new ->i_state accessors
xfs: use the new ->i_state accessors
ext4: use the new ->i_state accessors
f2fs: use the new ->i_state accessors
ceph: use the new ->i_state accessors
overlayfs: use the new ->i_state accessors
fs: make plain ->i_state access fail to compile
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 | 104 +++++++++++++-------------
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 | 70 +++++++++++++++++-
include/linux/writeback.h | 4 +-
include/trace/events/writeback.h | 8 +-
mm/backing-dev.c | 2 +-
security/landlock/fs.c | 2 +-
106 files changed, 371 insertions(+), 310 deletions(-)
--
2.43.0
next reply other threads:[~2025-09-16 13:59 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-16 13:58 Mateusz Guzik [this message]
2025-09-16 13:58 ` [PATCH v4 01/12] fs: provide accessors for ->i_state Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 02/12] fs: use ->i_state accessors in core kernel Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 03/12] fs: mechanically convert most filesystems to use ->i_state accessors Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 04/12] btrfs: use the new " Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 05/12] netfs: " Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 06/12] nilfs2: " Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 07/12] xfs: " Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 08/12] ext4: " Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 09/12] f2fs: " Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 10/12] ceph: " Mateusz Guzik
2025-09-16 17:36 ` Viacheslav Dubeyko
2025-09-16 17:38 ` Mateusz Guzik
2025-09-16 13:58 ` [PATCH v4 11/12] overlayfs: " Mateusz Guzik
2025-09-16 13:59 ` [PATCH v4 12/12] fs: make plain ->i_state access fail to compile Mateusz Guzik
2025-09-19 12:19 ` [PATCH v4 00/12] hide ->i_state behind accessors Christian Brauner
2025-09-19 13:09 ` Mateusz Guzik
2025-09-19 13:39 ` Mateusz Guzik
2025-09-19 15:51 ` Mateusz Guzik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250916135900.2170346-1-mjguzik@gmail.com \
--to=mjguzik@gmail.com \
--cc=amir73il@gmail.com \
--cc=brauner@kernel.org \
--cc=ceph-devel@vger.kernel.org \
--cc=jack@suse.cz \
--cc=josef@toxicpanda.com \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).