Re: [PATCH] fuse: fix inode initialization race

[Bernd Schubert]

On 3/26/26 15:26, Christian Brauner wrote:
> On Wed, Mar 25, 2026 at 08:54:57AM +0100, Bernd Schubert wrote:
>>
>>
>> On 3/18/26 14:43, Horst Birthelmer wrote:
>>> From: Horst Birthelmer <hbirthelmer@xxxxxxx>
>>>
>>> Fix a race between fuse_iget() and fuse_reverse_inval_inode() where
>>> invalidation can arrive while an inode is being initialized, causing
>>> the invalidation to be lost.
>>>
>>> Add a waitqueue to make fuse_reverse_inval_inode() wait when it
>>> encounters an inode with attr_version == 0 (still initializing).
>>> When fuse_change_attributes_common() completes initialization, it
>>> wakes waiting threads.
>>>
>>> This ensures invalidations are properly serialized with inode
>>> initialization, maintaining cache coherency.
>>>
>>> Signed-off-by: Horst Birthelmer <hbirthelmer@xxxxxxx>
>>> ---
>>>  fs/fuse/fuse_i.h | 3 +++
>>>  fs/fuse/inode.c  | 8 ++++++++
>>>  2 files changed, 11 insertions(+)
>>>
>>> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
>>> index 7f16049387d15e869db4be23a93605098588eda9..1be611472eee276371b3bde1a55257c1116cfedd 100644
>>> --- a/fs/fuse/fuse_i.h
>>> +++ b/fs/fuse/fuse_i.h
>>> @@ -945,6 +945,9 @@ struct fuse_conn {
>>>  	/** Version counter for attribute changes */
>>>  	atomic64_t attr_version;
>>>  
>>> +	/** Waitqueue for attr_version initialization */
>>> +	wait_queue_head_t attr_version_waitq;
>>> +
>>>  	/** Version counter for evict inode */
>>>  	atomic64_t evict_ctr;
>>>  
>>> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
>>> index e57b8af06be93ecc29c58864a9c9e99c68e3283b..c6e7e50d80c0edaea57d9342869eaf811786e342 100644
>>> --- a/fs/fuse/inode.c
>>> +++ b/fs/fuse/inode.c
>>> @@ -246,6 +246,7 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
>>>  		set_mask_bits(&fi->inval_mask, STATX_BASIC_STATS, 0);
>>>  
>>>  	fi->attr_version = atomic64_inc_return(&fc->attr_version);
>>> +	wake_up_all(&fc->attr_version_waitq);
>>>  	fi->i_time = attr_valid;


While I'm looking at this again, wouldn't it make sense to make this
conditional? Because we wake this queue on every attr change for every
inode. And the conditional in fuse_iget() based on I_NEW?



Thanks,
Bernd