On Wed, Sep 10, 2025 at 11:19 PM Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > On Wed, Sep 10, 2025 at 11:40:59AM +0200, Mateusz Guzik wrote: > > Normally all changes to ->i_state requires the ->i_lock spinlock held. > > > > The flag initially shows up in xfs_rename_alloc_whiteout, where I > > presume the inode is not visible to anyone else yet. > > > > It gets removed later in xfs_rename. > > > > I can't tell whether there is anyone else at that point who can mess > > with ->i_state. > > No-one else can find it because the directory the whiteout was added > to is still locked. Hence any readdir or lookup operation that might > expose the new whiteout inode to users will block on the directory > lock until after the rename transaction commits. > > We really don't care if the inode->i_lock is needed or not. We can > add it if necessary, but it's pure overhead for no actual gain. > Ok, I'll add a variant which foregoes the assertion. > > The context is that I'm writing a patchset which hides all ->i_state > > accesses behind helpers. Part of the functionality is to assert that > > the lock is held for changes of the sort. > > Yup, the version I looked at yesterday was .... too ugly to > consider. If I've got time, I'll comment there... > I'll be sending an updated patch today or tomorrow. -- Mateusz Guzik <mjguzik gmail.com>
