[PATCH] ceph: refactor wake_up_bit() pattern of calling

[PATCH] ceph: refactor wake_up_bit() pattern of calling

[Viacheslav Dubeyko]

From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>

The wake_up_bit() is called in ceph_async_unlink_cb(),
wake_async_create_waiters(), and ceph_finish_async_create().
It makes sense to switch on clear_bit() function, because
it makes the code much cleaner and easier to understand.
More important rework is the adding of smp_mb__after_atomic()
memory barrier after the bit modification and before
wake_up_bit() call. It can prevent potential race condition
of accessing the modified bit in other threads.

Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
---
 fs/ceph/dir.c  | 4 +++-
 fs/ceph/file.c | 8 ++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index a321aa6d0ed2..7f4d1874a84f 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1261,7 +1261,9 @@ static void ceph_async_unlink_cb(struct ceph_mds_client *mdsc,
 	spin_unlock(&fsc->async_unlink_conflict_lock);
 
 	spin_lock(&dentry->d_lock);
-	di->flags &= ~CEPH_DENTRY_ASYNC_UNLINK;
+	clear_bit(CEPH_DENTRY_ASYNC_UNLINK_BIT, &di->flags);
+	/* ensure modified bit is visible */
+	smp_mb__after_atomic();
 	wake_up_bit(&di->flags, CEPH_DENTRY_ASYNC_UNLINK_BIT);
 	spin_unlock(&dentry->d_lock);
 
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index a7254cab44cc..b114b939cdc0 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -580,7 +580,9 @@ static void wake_async_create_waiters(struct inode *inode,
 
 	spin_lock(&ci->i_ceph_lock);
 	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE) {
-		ci->i_ceph_flags &= ~CEPH_I_ASYNC_CREATE;
+		clear_bit(CEPH_ASYNC_CREATE_BIT, &ci->i_ceph_flags);
+		/* ensure modified bit is visible */
+		smp_mb__after_atomic();
 		wake_up_bit(&ci->i_ceph_flags, CEPH_ASYNC_CREATE_BIT);
 
 		if (ci->i_ceph_flags & CEPH_I_ASYNC_CHECK_CAPS) {
@@ -765,7 +767,9 @@ static int ceph_finish_async_create(struct inode *dir, struct inode *inode,
 	}
 
 	spin_lock(&dentry->d_lock);
-	di->flags &= ~CEPH_DENTRY_ASYNC_CREATE;
+	clear_bit(CEPH_DENTRY_ASYNC_CREATE_BIT, &di->flags);
+	/* ensure modified bit is visible */
+	smp_mb__after_atomic();
 	wake_up_bit(&di->flags, CEPH_DENTRY_ASYNC_CREATE_BIT);
 	spin_unlock(&dentry->d_lock);
 
-- 
2.49.0

Re: [PATCH] ceph: refactor wake_up_bit() pattern of calling

[Matthew Wilcox]

On Mon, Jul 07, 2025 at 01:03:22PM -0700, Viacheslav Dubeyko wrote:
>  	spin_lock(&dentry->d_lock);
> -	di->flags &= ~CEPH_DENTRY_ASYNC_UNLINK;
> +	clear_bit(CEPH_DENTRY_ASYNC_UNLINK_BIT, &di->flags);
> +	/* ensure modified bit is visible */
> +	smp_mb__after_atomic();
>  	wake_up_bit(&di->flags, CEPH_DENTRY_ASYNC_UNLINK_BIT);

Seems like you're open-coding clear_and_wake_up_bit()?

RE: [PATCH] ceph: refactor wake_up_bit() pattern of calling

[Viacheslav Dubeyko]

On Mon, 2025-07-07 at 21:27 +0100, Matthew Wilcox wrote:
> On Mon, Jul 07, 2025 at 01:03:22PM -0700, Viacheslav Dubeyko wrote:
> >  	spin_lock(&dentry->d_lock);
> > -	di->flags &= ~CEPH_DENTRY_ASYNC_UNLINK;
> > +	clear_bit(CEPH_DENTRY_ASYNC_UNLINK_BIT, &di->flags);
> > +	/* ensure modified bit is visible */
> > +	smp_mb__after_atomic();
> >  	wake_up_bit(&di->flags, CEPH_DENTRY_ASYNC_UNLINK_BIT);
> 
> Seems like you're open-coding clear_and_wake_up_bit()?

Damn, we already have this primitive. :) Makes sense to rework the patch.

Thanks,
Slava.