public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fuse: fix inode initialization race
@ 2026-03-18 13:43 Horst Birthelmer
  2026-03-25  7:54 ` Bernd Schubert
  2026-03-26 14:51 ` Miklos Szeredi
  0 siblings, 2 replies; 6+ messages in thread
From: Horst Birthelmer @ 2026-03-18 13:43 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Bernd Schubert, linux-fsdevel, linux-kernel, Horst Birthelmer

From: Horst Birthelmer <hbirthelmer@ddn.com>

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@ddn.com>
---
 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;
 
 	inode->i_ino     = fuse_squash_ino(attr->ino);
@@ -567,6 +568,12 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid,
 
 	fi = get_fuse_inode(inode);
 	spin_lock(&fi->lock);
+	while (fi->attr_version == 0) {
+		spin_unlock(&fi->lock);
+		wait_event(fc->attr_version_waitq, READ_ONCE(fi->attr_version) != 0);
+		spin_lock(&fi->lock);
+	}
+
 	fi->attr_version = atomic64_inc_return(&fc->attr_version);
 	spin_unlock(&fi->lock);
 
@@ -979,6 +986,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 	atomic_set(&fc->epoch, 1);
 	INIT_WORK(&fc->epoch_work, fuse_epoch_work);
 	init_waitqueue_head(&fc->blocked_waitq);
+	init_waitqueue_head(&fc->attr_version_waitq);
 	fuse_iqueue_init(&fc->iq, fiq_ops, fiq_priv);
 	INIT_LIST_HEAD(&fc->bg_queue);
 	INIT_LIST_HEAD(&fc->entry);

---
base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
change-id: 20260318-fix-inode-init-race-a47a7ba4af1e

Best regards,
-- 
Horst Birthelmer <hbirthelmer@ddn.com>


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] fuse: fix inode initialization race
  2026-03-18 13:43 [PATCH] fuse: fix inode initialization race Horst Birthelmer
@ 2026-03-25  7:54 ` Bernd Schubert
  2026-03-26 14:26   ` Christian Brauner
  2026-03-26 14:51 ` Miklos Szeredi
  1 sibling, 1 reply; 6+ messages in thread
From: Bernd Schubert @ 2026-03-25  7:54 UTC (permalink / raw)
  To: Horst Birthelmer, Miklos Szeredi
  Cc: linux-fsdevel, linux-kernel, Horst Birthelmer



On 3/18/26 14:43, Horst Birthelmer wrote:
> From: Horst Birthelmer <hbirthelmer@ddn.com>
> 
> 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@ddn.com>
> ---
>  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;
>  
>  	inode->i_ino     = fuse_squash_ino(attr->ino);
> @@ -567,6 +568,12 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid,
>  
>  	fi = get_fuse_inode(inode);
>  	spin_lock(&fi->lock);
> +	while (fi->attr_version == 0) {
> +		spin_unlock(&fi->lock);
> +		wait_event(fc->attr_version_waitq, READ_ONCE(fi->attr_version) != 0);
> +		spin_lock(&fi->lock);
> +	}
> +
>  	fi->attr_version = atomic64_inc_return(&fc->attr_version);
>  	spin_unlock(&fi->lock);
>  
> @@ -979,6 +986,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
>  	atomic_set(&fc->epoch, 1);
>  	INIT_WORK(&fc->epoch_work, fuse_epoch_work);
>  	init_waitqueue_head(&fc->blocked_waitq);
> +	init_waitqueue_head(&fc->attr_version_waitq);
>  	fuse_iqueue_init(&fc->iq, fiq_ops, fiq_priv);
>  	INIT_LIST_HEAD(&fc->bg_queue);
>  	INIT_LIST_HEAD(&fc->entry);
> 
> ---
> base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
> change-id: 20260318-fix-inode-init-race-a47a7ba4af1e
> 
> Best regards,

Had reviewed that DDN internally already. LGTM

Reviewed-by: Bernd Schubert <bernd@bsbernd.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fuse: fix inode initialization race
  2026-03-25  7:54 ` Bernd Schubert
@ 2026-03-26 14:26   ` Christian Brauner
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-03-26 14:26 UTC (permalink / raw)
  To: Bernd Schubert
  Cc: Horst Birthelmer, Miklos Szeredi, linux-fsdevel, linux-kernel,
	Horst Birthelmer

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@ddn.com>
> > 
> > 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@ddn.com>
> > ---
> >  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;
> >  
> >  	inode->i_ino     = fuse_squash_ino(attr->ino);
> > @@ -567,6 +568,12 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid,
> >  
> >  	fi = get_fuse_inode(inode);
> >  	spin_lock(&fi->lock);
> > +	while (fi->attr_version == 0) {
> > +		spin_unlock(&fi->lock);
> > +		wait_event(fc->attr_version_waitq, READ_ONCE(fi->attr_version) != 0);
> > +		spin_lock(&fi->lock);
> > +	}
> > +
> >  	fi->attr_version = atomic64_inc_return(&fc->attr_version);
> >  	spin_unlock(&fi->lock);
> >  
> > @@ -979,6 +986,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
> >  	atomic_set(&fc->epoch, 1);
> >  	INIT_WORK(&fc->epoch_work, fuse_epoch_work);
> >  	init_waitqueue_head(&fc->blocked_waitq);
> > +	init_waitqueue_head(&fc->attr_version_waitq);
> >  	fuse_iqueue_init(&fc->iq, fiq_ops, fiq_priv);
> >  	INIT_LIST_HEAD(&fc->bg_queue);
> >  	INIT_LIST_HEAD(&fc->entry);
> > 
> > ---
> > base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
> > change-id: 20260318-fix-inode-init-race-a47a7ba4af1e
> > 
> > Best regards,
> 
> Had reviewed that DDN internally already. LGTM
> 
> Reviewed-by: Bernd Schubert <bernd@bsbernd.com>

Should I grab it?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fuse: fix inode initialization race
  2026-03-18 13:43 [PATCH] fuse: fix inode initialization race Horst Birthelmer
  2026-03-25  7:54 ` Bernd Schubert
@ 2026-03-26 14:51 ` Miklos Szeredi
  2026-03-26 14:56   ` Horst Birthelmer
  1 sibling, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2026-03-26 14:51 UTC (permalink / raw)
  To: Horst Birthelmer
  Cc: Bernd Schubert, linux-fsdevel, linux-kernel, Horst Birthelmer

On Wed, 18 Mar 2026 at 14:45, Horst Birthelmer <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> 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 should be relatively rare, right?  In that case a single global
waitq and wake_up_all() would be better, imo.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: [PATCH] fuse: fix inode initialization race
  2026-03-26 14:51 ` Miklos Szeredi
@ 2026-03-26 14:56   ` Horst Birthelmer
  2026-03-26 15:06     ` Miklos Szeredi
  0 siblings, 1 reply; 6+ messages in thread
From: Horst Birthelmer @ 2026-03-26 14:56 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Horst Birthelmer, Bernd Schubert, linux-fsdevel, linux-kernel,
	Horst Birthelmer

On Thu, Mar 26, 2026 at 03:51:18PM +0100, Miklos Szeredi wrote:
> On Wed, 18 Mar 2026 at 14:45, Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > 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 should be relatively rare, right?  In that case a single global
> waitq and wake_up_all() would be better, imo.

Well it depends on the use case. We send relatively many notifications
since they are bound to the DLM system and thus to changes done by a lot
of clients and so it happens that you get an invalidation while still
creating the inode.

What is wrong with one per connection?

> 
> Thanks,
> Miklos

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: [PATCH] fuse: fix inode initialization race
  2026-03-26 14:56   ` Horst Birthelmer
@ 2026-03-26 15:06     ` Miklos Szeredi
  0 siblings, 0 replies; 6+ messages in thread
From: Miklos Szeredi @ 2026-03-26 15:06 UTC (permalink / raw)
  To: Horst Birthelmer
  Cc: Horst Birthelmer, Bernd Schubert, linux-fsdevel, linux-kernel,
	Horst Birthelmer

On Thu, 26 Mar 2026 at 15:57, Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Thu, Mar 26, 2026 at 03:51:18PM +0100, Miklos Szeredi wrote:
> > On Wed, 18 Mar 2026 at 14:45, Horst Birthelmer <horst@birthelmer.com> wrote:
> > >
> > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > >
> > > 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 should be relatively rare, right?  In that case a single global
> > waitq and wake_up_all() would be better, imo.
>
> Well it depends on the use case. We send relatively many notifications
> since they are bound to the DLM system and thus to changes done by a lot
> of clients and so it happens that you get an invalidation while still
> creating the inode.
>
> What is wrong with one per connection?

It seemed to be something that would be very rarely used, hence having
a waitq_head per fc is not space efficient.

If two such events are likely to collide multiple times per second,
then I have nothing against a per-fc waitq, otherwise a global one
will be just as good.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-26 15:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-18 13:43 [PATCH] fuse: fix inode initialization race Horst Birthelmer
2026-03-25  7:54 ` Bernd Schubert
2026-03-26 14:26   ` Christian Brauner
2026-03-26 14:51 ` Miklos Szeredi
2026-03-26 14:56   ` Horst Birthelmer
2026-03-26 15:06     ` Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox