Good cleanup.
Reviewed-by: Alex Markuze amarkuze@xxxxxxxxxx
On Tue, Jul 8, 2025 at 10:21 PM Viacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:
>
> From: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
>
> 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. Luckily,
> clear_and_wake_up_bit() already implements the required
> functionality pattern:
>
> static inline void clear_and_wake_up_bit(int bit, unsigned long *word)
> {
> clear_bit_unlock(bit, word);
> /* See wake_up_bit() for which memory barrier you need to use. */
> smp_mb__after_atomic();
> wake_up_bit(word, bit);
> }
>
> Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
> ---
> fs/ceph/dir.c | 3 +--
> fs/ceph/file.c | 6 ++----
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index a321aa6d0ed2..1535b6540e9d 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1261,8 +1261,7 @@ 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;
> - wake_up_bit(&di->flags, CEPH_DENTRY_ASYNC_UNLINK_BIT);
> + clear_and_wake_up_bit(CEPH_DENTRY_ASYNC_UNLINK_BIT, &di->flags);
> spin_unlock(&dentry->d_lock);
>
> synchronize_rcu();
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index a7254cab44cc..57451958624e 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -580,8 +580,7 @@ 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;
> - wake_up_bit(&ci->i_ceph_flags, CEPH_ASYNC_CREATE_BIT);
> + clear_and_wake_up_bit(CEPH_ASYNC_CREATE_BIT, &ci->i_ceph_flags);
>
> if (ci->i_ceph_flags & CEPH_I_ASYNC_CHECK_CAPS) {
> ci->i_ceph_flags &= ~CEPH_I_ASYNC_CHECK_CAPS;
> @@ -765,8 +764,7 @@ static int ceph_finish_async_create(struct inode *dir, struct inode *inode,
> }
>
> spin_lock(&dentry->d_lock);
> - di->flags &= ~CEPH_DENTRY_ASYNC_CREATE;
> - wake_up_bit(&di->flags, CEPH_DENTRY_ASYNC_CREATE_BIT);
> + clear_and_wake_up_bit(CEPH_DENTRY_ASYNC_CREATE_BIT, &di->flags);
> spin_unlock(&dentry->d_lock);
>
> return ret;
> --
> 2.49.0
>
