Thread (11 messages) 11 messages, 3 authors, 2025-09-22

Re: [PATCH v5 0/4] hide ->i_state behind accessors

From: Mateusz Guzik <hidden>
Date: 2025-09-20 05:48:00
Also in: ceph-devel, linux-btrfs, linux-fsdevel, linux-unionfs, linux-xfs, lkml

On Sat, Sep 20, 2025 at 6:31 AM Russell Haley [off-list ref] wrote:
On 9/19/25 10:49 AM, Mateusz Guzik wrote:
quoted
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).

Given the late stage of the release cycle this patchset only aims to
hide access, it does not provide any of the checks.

Consumers can be trivially converted. Suppose flags I_A and I_B are to
be handled, then:

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)
[/quote]
Drive-by bikeshedding: s/set/replace/g

"replace" removes ambiguity with the concept of setting a bit ( |= ). An
alternative would be "set_only".
I agree _set may be ambiguous here. I was considering something like
_assign or _set_value instead.

I'm not that fond of _replace but I'm not going to really going to
argue about any particular variant.

The good news is that whatever the naming, sed indeed can be used to
adjust the patchset. :)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help