Skip to content

Commit 11f2af2

Browse files
braunerbrauner
committed
Merge patch series "hide ->i_state behind accessors"
Mateusz Guzik <mjguzik@gmail.com> says: 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. If ->i_lock is held, then: state = inode->i_state => state = inode_state_read(inode) inode->i_state |= (I_A | I_B) => inode_state_set(inode, I_A | I_B) inode->i_state &= ~(I_A | I_B) => inode_state_clear(inode, I_A | I_B) inode->i_state = I_A | I_B => inode_state_assign(inode, I_A | I_B) If ->i_lock is not held or only held conditionally: state = inode->i_state => state = inode_state_read_once(inode) inode->i_state |= (I_A | I_B) => inode_state_set_raw(inode, I_A | I_B) inode->i_state &= ~(I_A | I_B) => inode_state_clear_raw(inode, I_A | I_B) inode->i_state = I_A | I_B => inode_state_assign_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. Finally, if you want to atomically clear flags and set new ones, the following: state = inode->i_state; state &= ~I_A; state |= I_B; inode->i_state = state; turns into: inode_state_replace(inode, I_A, I_B); * patches from https://lore.kernel.org/20251009075929.1203950-1-mjguzik@gmail.com: fs: make plain ->i_state access fail to compile xfs: use the new ->i_state accessors nilfs2: use the new ->i_state accessors overlayfs: use the new ->i_state accessors gfs2: use the new ->i_state accessors f2fs: use the new ->i_state accessors smb: use the new ->i_state accessors ceph: use the new ->i_state accessors btrfs: use the new ->i_state accessors Manual conversion to use ->i_state accessors of all places not covered by coccinelle Coccinelle-based conversion to use ->i_state accessors fs: provide accessors for ->i_state fs: spell out fenced ->i_state accesses with explicit smp_wmb/smp_rmb fs: move wait_on_inode() from writeback.h to fs.h Signed-off-by: Christian Brauner <brauner@kernel.org>
2 parents 31e332b + 2ed81b4 commit 11f2af2

106 files changed

Lines changed: 395 additions & 315 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Documentation/filesystems/porting.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ test and set for you.
211211
e.g.::
212212

213213
inode = iget_locked(sb, ino);
214-
if (inode->i_state & I_NEW) {
214+
if (inode_state_read_once(inode) & I_NEW) {
215215
err = read_inode_from_disk(inode);
216216
if (err < 0) {
217217
iget_failed(inode);

‎block/bdev.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ static void bdev_write_inode(struct block_device *bdev)
6767
int ret;
6868

6969
spin_lock(&inode->i_lock);
70-
while (inode->i_state & I_DIRTY) {
70+
while (inode_state_read(inode) & I_DIRTY) {
7171
spin_unlock(&inode->i_lock);
7272
ret = write_inode_now(inode, true);
7373
if (ret)
@@ -1265,7 +1265,7 @@ void sync_bdevs(bool wait)
12651265
struct block_device *bdev;
12661266

12671267
spin_lock(&inode->i_lock);
1268-
if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW) ||
1268+
if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW) ||
12691269
mapping->nrpages == 0) {
12701270
spin_unlock(&inode->i_lock);
12711271
continue;

‎drivers/dax/super.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ static struct dax_device *dax_dev_get(dev_t devt)
433433
return NULL;
434434

435435
dax_dev = to_dax_dev(inode);
436-
if (inode->i_state & I_NEW) {
436+
if (inode_state_read_once(inode) & I_NEW) {
437437
set_bit(DAXDEV_ALIVE, &dax_dev->flags);
438438
inode->i_cdev = &dax_dev->cdev;
439439
inode->i_mode = S_IFCHR;

‎fs/9p/vfs_inode.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
422422
inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode, st);
423423
if (!inode)
424424
return ERR_PTR(-ENOMEM);
425-
if (!(inode->i_state & I_NEW))
425+
if (!(inode_state_read_once(inode) & I_NEW))
426426
return inode;
427427
/*
428428
* initialize the inode with the stat info

‎fs/9p/vfs_inode_dotl.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
112112
inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode_dotl, st);
113113
if (!inode)
114114
return ERR_PTR(-ENOMEM);
115-
if (!(inode->i_state & I_NEW))
115+
if (!(inode_state_read_once(inode) & I_NEW))
116116
return inode;
117117
/*
118118
* initialize the inode with the stat info

‎fs/affs/inode.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ struct inode *affs_iget(struct super_block *sb, unsigned long ino)
2929
inode = iget_locked(sb, ino);
3030
if (!inode)
3131
return ERR_PTR(-ENOMEM);
32-
if (!(inode->i_state & I_NEW))
32+
if (!(inode_state_read_once(inode) & I_NEW))
3333
return inode;
3434

3535
pr_debug("affs_iget(%lu)\n", inode->i_ino);

‎fs/afs/dynroot.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ static struct inode *afs_iget_pseudo_dir(struct super_block *sb, ino_t ino)
6464

6565
vnode = AFS_FS_I(inode);
6666

67-
if (inode->i_state & I_NEW) {
67+
if (inode_state_read_once(inode) & I_NEW) {
6868
netfs_inode_init(&vnode->netfs, NULL, false);
6969
simple_inode_init_ts(inode);
7070
set_nlink(inode, 2);
@@ -258,7 +258,7 @@ static struct dentry *afs_lookup_atcell(struct inode *dir, struct dentry *dentry
258258

259259
vnode = AFS_FS_I(inode);
260260

261-
if (inode->i_state & I_NEW) {
261+
if (inode_state_read_once(inode) & I_NEW) {
262262
netfs_inode_init(&vnode->netfs, NULL, false);
263263
simple_inode_init_ts(inode);
264264
set_nlink(inode, 1);
@@ -383,7 +383,7 @@ struct inode *afs_dynroot_iget_root(struct super_block *sb)
383383
vnode = AFS_FS_I(inode);
384384

385385
/* there shouldn't be an existing inode */
386-
if (inode->i_state & I_NEW) {
386+
if (inode_state_read_once(inode) & I_NEW) {
387387
netfs_inode_init(&vnode->netfs, NULL, false);
388388
simple_inode_init_ts(inode);
389389
set_nlink(inode, 2);

‎fs/afs/inode.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ static void afs_fetch_status_success(struct afs_operation *op)
427427
struct afs_vnode *vnode = vp->vnode;
428428
int ret;
429429

430-
if (vnode->netfs.inode.i_state & I_NEW) {
430+
if (inode_state_read_once(&vnode->netfs.inode) & I_NEW) {
431431
ret = afs_inode_init_from_status(op, vp, vnode);
432432
afs_op_set_error(op, ret);
433433
if (ret == 0)
@@ -579,7 +579,7 @@ struct inode *afs_iget(struct afs_operation *op, struct afs_vnode_param *vp)
579579
inode, vnode->fid.vid, vnode->fid.vnode, vnode->fid.unique);
580580

581581
/* deal with an existing inode */
582-
if (!(inode->i_state & I_NEW)) {
582+
if (!(inode_state_read_once(inode) & I_NEW)) {
583583
_leave(" = %p", inode);
584584
return inode;
585585
}
@@ -639,7 +639,7 @@ struct inode *afs_root_iget(struct super_block *sb, struct key *key)
639639

640640
_debug("GOT ROOT INODE %p { vl=%llx }", inode, as->volume->vid);
641641

642-
BUG_ON(!(inode->i_state & I_NEW));
642+
BUG_ON(!(inode_state_read_once(inode) & I_NEW));
643643

644644
vnode = AFS_FS_I(inode);
645645
vnode->cb_v_check = atomic_read(&as->volume->cb_v_break);
@@ -748,7 +748,7 @@ void afs_evict_inode(struct inode *inode)
748748

749749
if ((S_ISDIR(inode->i_mode) ||
750750
S_ISLNK(inode->i_mode)) &&
751-
(inode->i_state & I_DIRTY) &&
751+
(inode_state_read_once(inode) & I_DIRTY) &&
752752
!sbi->dyn_root) {
753753
struct writeback_control wbc = {
754754
.sync_mode = WB_SYNC_ALL,

‎fs/befs/linuxvfs.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
307307
inode = iget_locked(sb, ino);
308308
if (!inode)
309309
return ERR_PTR(-ENOMEM);
310-
if (!(inode->i_state & I_NEW))
310+
if (!(inode_state_read_once(inode) & I_NEW))
311311
return inode;
312312

313313
befs_ino = BEFS_I(inode);

‎fs/bfs/inode.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ struct inode *bfs_iget(struct super_block *sb, unsigned long ino)
4242
inode = iget_locked(sb, ino);
4343
if (!inode)
4444
return ERR_PTR(-ENOMEM);
45-
if (!(inode->i_state & I_NEW))
45+
if (!(inode_state_read_once(inode) & I_NEW))
4646
return inode;
4747

4848
if ((ino < BFS_ROOT_INO) || (ino > BFS_SB(inode->i_sb)->si_lasti)) {

0 commit comments

Comments
 (0)