[PATCH 0/2] lockref tidy ups + touch ups to it's usage by dcache

[PATCH 0/2] lockref tidy ups + touch ups to it's usage by dcache

[Mateusz Guzik]

The open-coded check for < 0 indicating a dead object uses information
it should not know, so to speak.

This is a preliminary clean up for a longer term goal of moving dcache
away from lockref into an approach which can atomic_add both ways
instead of suffering the cmpxchg loop.

Mateusz Guzik (2):
  lockref: tidy up dead count handling
  dcache: use lockref routines for dead count checks

 fs/ceph/dir.c           |  2 +-
 fs/dcache.c             | 16 ++++++++--------
 fs/erofs/zdata.c        |  4 ++--
 fs/gfs2/glock.c         |  6 +++---
 fs/gfs2/lock_dlm.c      |  6 +++---
 fs/gfs2/quota.c         |  4 ++--
 fs/xfs/xfs_buf.c        |  4 ++--
 fs/xfs/xfs_qm.c         |  4 ++--
 include/linux/lockref.h | 12 ++++++++++--
 lib/lockref.c           |  2 +-
 10 files changed, 34 insertions(+), 26 deletions(-)

-- 
2.48.1

[PATCH 1/2] lockref: tidy up dead count handling

[Mateusz Guzik]

1. put the dead val into a macro so that it can be used in other places
2. __lockref_is_dead():
- drop the __ suffix, this is not an internal routine
- drop the spurious cast, the value is already a signed int
- use READ_ONCE to prevent any compile shenanigans
3. provide lockref_is_dead_or_zero()

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/ceph/dir.c           |  2 +-
 fs/erofs/zdata.c        |  4 ++--
 fs/gfs2/glock.c         |  6 +++---
 fs/gfs2/lock_dlm.c      |  6 +++---
 fs/gfs2/quota.c         |  4 ++--
 fs/xfs/xfs_buf.c        |  4 ++--
 fs/xfs/xfs_qm.c         |  4 ++--
 include/linux/lockref.h | 12 ++++++++++--
 lib/lockref.c           |  2 +-
 9 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index b957149abe5a..f39856ff3ea1 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1667,7 +1667,7 @@ __dentry_leases_walk(struct ceph_mds_client *mdsc,
 		if (!spin_trylock(&dentry->d_lock))
 			continue;
 
-		if (__lockref_is_dead(&dentry->d_lockref)) {
+		if (lockref_is_dead(&dentry->d_lockref)) {
 			list_del_init(&di->lease_list);
 			goto next;
 		}
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index 74520e910259..d022d1dff5a1 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -725,7 +725,7 @@ static bool z_erofs_get_pcluster(struct z_erofs_pcluster *pcl)
 		return true;
 
 	spin_lock(&pcl->lockref.lock);
-	if (__lockref_is_dead(&pcl->lockref)) {
+	if (lockref_is_dead(&pcl->lockref)) {
 		spin_unlock(&pcl->lockref.lock);
 		return false;
 	}
@@ -945,7 +945,7 @@ static void z_erofs_put_pcluster(struct erofs_sb_info *sbi,
 	if (lockref_put_or_lock(&pcl->lockref))
 		return;
 
-	DBG_BUGON(__lockref_is_dead(&pcl->lockref));
+	DBG_BUGON(lockref_is_dead(&pcl->lockref));
 	if (!--pcl->lockref.count) {
 		if (try_free && xa_trylock(&sbi->managed_pslots)) {
 			free = __erofs_try_to_release_pcluster(sbi, pcl);
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index b8a144d3a73b..eaa2980051ed 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -2080,7 +2080,7 @@ static void clear_glock(struct gfs2_glock *gl)
 	gfs2_glock_remove_from_lru(gl);
 
 	spin_lock(&gl->gl_lockref.lock);
-	if (!__lockref_is_dead(&gl->gl_lockref)) {
+	if (!lockref_is_dead(&gl->gl_lockref)) {
 		gl->gl_lockref.count++;
 		if (gl->gl_state != LM_ST_UNLOCKED)
 			request_demote(gl, LM_ST_UNLOCKED, 0, false);
@@ -2115,7 +2115,7 @@ static void dump_glock_func(struct gfs2_glock *gl)
 static void withdraw_glock(struct gfs2_glock *gl)
 {
 	spin_lock(&gl->gl_lockref.lock);
-	if (!__lockref_is_dead(&gl->gl_lockref)) {
+	if (!lockref_is_dead(&gl->gl_lockref)) {
 		/*
 		 * We don't want to write back any more dirty data.  Unlock the
 		 * remaining inode and resource group glocks; this will cause
@@ -2483,7 +2483,7 @@ static void gfs2_glock_iter_next(struct gfs2_glock_iter *gi, loff_t n)
 				continue;
 			break;
 		} else {
-			if (__lockref_is_dead(&gl->gl_lockref))
+			if (lockref_is_dead(&gl->gl_lockref))
 				continue;
 			n--;
 		}
diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c
index 7828ad0b6f5a..cc901fb97da0 100644
--- a/fs/gfs2/lock_dlm.c
+++ b/fs/gfs2/lock_dlm.c
@@ -126,7 +126,7 @@ static void gdlm_ast(void *arg)
 	clear_bit(GLF_BLOCKING, &gl->gl_flags);
 
 	/* If the glock is dead, we only react to a dlm_unlock() reply. */
-	if (__lockref_is_dead(&gl->gl_lockref) &&
+	if (lockref_is_dead(&gl->gl_lockref) &&
 	    gl->gl_lksb.sb_status != -DLM_EUNLOCK)
 		return;
 
@@ -182,7 +182,7 @@ static void gdlm_bast(void *arg, int mode)
 {
 	struct gfs2_glock *gl = arg;
 
-	if (__lockref_is_dead(&gl->gl_lockref))
+	if (lockref_is_dead(&gl->gl_lockref))
 		return;
 
 	switch (mode) {
@@ -329,7 +329,7 @@ static void gdlm_put_lock(struct gfs2_glock *gl)
 	uint32_t flags = 0;
 	int error;
 
-	BUG_ON(!__lockref_is_dead(&gl->gl_lockref));
+	BUG_ON(!lockref_is_dead(&gl->gl_lockref));
 
 	if (test_bit(GLF_INITIAL, &gl->gl_flags)) {
 		gfs2_glock_free(gl);
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 91e9975d25e8..001c8b39ca55 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -342,7 +342,7 @@ static void qd_put(struct gfs2_quota_data *qd)
 	if (lockref_put_or_lock(&qd->qd_lockref))
 		return;
 
-	BUG_ON(__lockref_is_dead(&qd->qd_lockref));
+	BUG_ON(lockref_is_dead(&qd->qd_lockref));
 	sdp = qd->qd_sbd;
 	if (unlikely(!test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags))) {
 		lockref_mark_dead(&qd->qd_lockref);
@@ -486,7 +486,7 @@ static bool qd_grab_sync(struct gfs2_sbd *sdp, struct gfs2_quota_data *qd,
 	    qd->qd_sync_gen >= sync_gen)
 		goto out;
 
-	if (__lockref_is_dead(&qd->qd_lockref))
+	if (lockref_is_dead(&qd->qd_lockref))
 		goto out;
 	qd->qd_lockref.count++;
 
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 21d67af781da..17b9d643e1a8 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -80,7 +80,7 @@ xfs_buf_stale(
 
 	spin_lock(&bp->b_lockref.lock);
 	atomic_set(&bp->b_lru_ref, 0);
-	if (!__lockref_is_dead(&bp->b_lockref))
+	if (!lockref_is_dead(&bp->b_lockref))
 		list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru);
 	spin_unlock(&bp->b_lockref.lock);
 }
@@ -841,7 +841,7 @@ static void
 xfs_buf_destroy(
 	struct xfs_buf		*bp)
 {
-	ASSERT(__lockref_is_dead(&bp->b_lockref));
+	ASSERT(lockref_is_dead(&bp->b_lockref));
 	ASSERT(!(bp->b_flags & _XBF_DELWRI_Q));
 
 	if (bp->b_pag)
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 896b24f87ac9..99a82107b8e6 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -128,7 +128,7 @@ xfs_qm_dqpurge(
 	struct xfs_quotainfo	*qi = dqp->q_mount->m_quotainfo;
 
 	spin_lock(&dqp->q_lockref.lock);
-	if (dqp->q_lockref.count > 0 || __lockref_is_dead(&dqp->q_lockref)) {
+	if (dqp->q_lockref.count > 0 || lockref_is_dead(&dqp->q_lockref)) {
 		spin_unlock(&dqp->q_lockref.lock);
 		return -EAGAIN;
 	}
@@ -429,7 +429,7 @@ xfs_qm_dquot_isolate(
 	 * from the LRU, leave it for the freeing task to complete the freeing
 	 * process rather than risk it being free from under us here.
 	 */
-	if (__lockref_is_dead(&dqp->q_lockref))
+	if (lockref_is_dead(&dqp->q_lockref))
 		goto out_miss_unlock;
 
 	/*
diff --git a/include/linux/lockref.h b/include/linux/lockref.h
index 6ded24cdb4a8..ddfb7d3b8cec 100644
--- a/include/linux/lockref.h
+++ b/include/linux/lockref.h
@@ -34,6 +34,8 @@ struct lockref {
 	};
 };
 
+#define __LOCKREF_DEAD_VAL	-128
+
 /**
  * lockref_init - Initialize a lockref
  * @lockref: pointer to lockref structure
@@ -55,9 +57,15 @@ void lockref_mark_dead(struct lockref *lockref);
 bool lockref_get_not_dead(struct lockref *lockref);
 
 /* Must be called under spinlock for reliable results */
-static inline bool __lockref_is_dead(const struct lockref *l)
+static inline bool lockref_is_dead(const struct lockref *l)
+{
+	return (READ_ONCE(l->count) == __LOCKREF_DEAD_VAL);
+}
+
+static inline bool lockref_is_dead_or_zero(const struct lockref *l)
 {
-	return ((int)l->count < 0);
+	int count = READ_ONCE(l->count);
+	return (count == __LOCKREF_DEAD_VAL || count == 0);
 }
 
 #endif /* __LINUX_LOCKREF_H */
diff --git a/lib/lockref.c b/lib/lockref.c
index 5d8e3ef3860e..9b3dd688d8cd 100644
--- a/lib/lockref.c
+++ b/lib/lockref.c
@@ -131,7 +131,7 @@ EXPORT_SYMBOL(lockref_put_or_lock);
 void lockref_mark_dead(struct lockref *lockref)
 {
 	assert_spin_locked(&lockref->lock);
-	lockref->count = -128;
+	lockref->count = __LOCKREF_DEAD_VAL;
 }
 EXPORT_SYMBOL(lockref_mark_dead);
 
-- 
2.48.1

Re: [PATCH 1/2] lockref: tidy up dead count handling

[Jan Kara]

On Fri 24-07-26 19:14:21, Mateusz Guzik wrote:
> 1. put the dead val into a macro so that it can be used in other places
> 2. __lockref_is_dead():
> - drop the __ suffix, this is not an internal routine
> - drop the spurious cast, the value is already a signed int
> - use READ_ONCE to prevent any compile shenanigans
> 3. provide lockref_is_dead_or_zero()
> 
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ceph/dir.c           |  2 +-
>  fs/erofs/zdata.c        |  4 ++--
>  fs/gfs2/glock.c         |  6 +++---
>  fs/gfs2/lock_dlm.c      |  6 +++---
>  fs/gfs2/quota.c         |  4 ++--
>  fs/xfs/xfs_buf.c        |  4 ++--
>  fs/xfs/xfs_qm.c         |  4 ++--
>  include/linux/lockref.h | 12 ++++++++++--
>  lib/lockref.c           |  2 +-
>  9 files changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index b957149abe5a..f39856ff3ea1 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1667,7 +1667,7 @@ __dentry_leases_walk(struct ceph_mds_client *mdsc,
>  		if (!spin_trylock(&dentry->d_lock))
>  			continue;
>  
> -		if (__lockref_is_dead(&dentry->d_lockref)) {
> +		if (lockref_is_dead(&dentry->d_lockref)) {
>  			list_del_init(&di->lease_list);
>  			goto next;
>  		}
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index 74520e910259..d022d1dff5a1 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -725,7 +725,7 @@ static bool z_erofs_get_pcluster(struct z_erofs_pcluster *pcl)
>  		return true;
>  
>  	spin_lock(&pcl->lockref.lock);
> -	if (__lockref_is_dead(&pcl->lockref)) {
> +	if (lockref_is_dead(&pcl->lockref)) {
>  		spin_unlock(&pcl->lockref.lock);
>  		return false;
>  	}
> @@ -945,7 +945,7 @@ static void z_erofs_put_pcluster(struct erofs_sb_info *sbi,
>  	if (lockref_put_or_lock(&pcl->lockref))
>  		return;
>  
> -	DBG_BUGON(__lockref_is_dead(&pcl->lockref));
> +	DBG_BUGON(lockref_is_dead(&pcl->lockref));
>  	if (!--pcl->lockref.count) {
>  		if (try_free && xa_trylock(&sbi->managed_pslots)) {
>  			free = __erofs_try_to_release_pcluster(sbi, pcl);
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index b8a144d3a73b..eaa2980051ed 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -2080,7 +2080,7 @@ static void clear_glock(struct gfs2_glock *gl)
>  	gfs2_glock_remove_from_lru(gl);
>  
>  	spin_lock(&gl->gl_lockref.lock);
> -	if (!__lockref_is_dead(&gl->gl_lockref)) {
> +	if (!lockref_is_dead(&gl->gl_lockref)) {
>  		gl->gl_lockref.count++;
>  		if (gl->gl_state != LM_ST_UNLOCKED)
>  			request_demote(gl, LM_ST_UNLOCKED, 0, false);
> @@ -2115,7 +2115,7 @@ static void dump_glock_func(struct gfs2_glock *gl)
>  static void withdraw_glock(struct gfs2_glock *gl)
>  {
>  	spin_lock(&gl->gl_lockref.lock);
> -	if (!__lockref_is_dead(&gl->gl_lockref)) {
> +	if (!lockref_is_dead(&gl->gl_lockref)) {
>  		/*
>  		 * We don't want to write back any more dirty data.  Unlock the
>  		 * remaining inode and resource group glocks; this will cause
> @@ -2483,7 +2483,7 @@ static void gfs2_glock_iter_next(struct gfs2_glock_iter *gi, loff_t n)
>  				continue;
>  			break;
>  		} else {
> -			if (__lockref_is_dead(&gl->gl_lockref))
> +			if (lockref_is_dead(&gl->gl_lockref))
>  				continue;
>  			n--;
>  		}
> diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c
> index 7828ad0b6f5a..cc901fb97da0 100644
> --- a/fs/gfs2/lock_dlm.c
> +++ b/fs/gfs2/lock_dlm.c
> @@ -126,7 +126,7 @@ static void gdlm_ast(void *arg)
>  	clear_bit(GLF_BLOCKING, &gl->gl_flags);
>  
>  	/* If the glock is dead, we only react to a dlm_unlock() reply. */
> -	if (__lockref_is_dead(&gl->gl_lockref) &&
> +	if (lockref_is_dead(&gl->gl_lockref) &&
>  	    gl->gl_lksb.sb_status != -DLM_EUNLOCK)
>  		return;
>  
> @@ -182,7 +182,7 @@ static void gdlm_bast(void *arg, int mode)
>  {
>  	struct gfs2_glock *gl = arg;
>  
> -	if (__lockref_is_dead(&gl->gl_lockref))
> +	if (lockref_is_dead(&gl->gl_lockref))
>  		return;
>  
>  	switch (mode) {
> @@ -329,7 +329,7 @@ static void gdlm_put_lock(struct gfs2_glock *gl)
>  	uint32_t flags = 0;
>  	int error;
>  
> -	BUG_ON(!__lockref_is_dead(&gl->gl_lockref));
> +	BUG_ON(!lockref_is_dead(&gl->gl_lockref));
>  
>  	if (test_bit(GLF_INITIAL, &gl->gl_flags)) {
>  		gfs2_glock_free(gl);
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index 91e9975d25e8..001c8b39ca55 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -342,7 +342,7 @@ static void qd_put(struct gfs2_quota_data *qd)
>  	if (lockref_put_or_lock(&qd->qd_lockref))
>  		return;
>  
> -	BUG_ON(__lockref_is_dead(&qd->qd_lockref));
> +	BUG_ON(lockref_is_dead(&qd->qd_lockref));
>  	sdp = qd->qd_sbd;
>  	if (unlikely(!test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags))) {
>  		lockref_mark_dead(&qd->qd_lockref);
> @@ -486,7 +486,7 @@ static bool qd_grab_sync(struct gfs2_sbd *sdp, struct gfs2_quota_data *qd,
>  	    qd->qd_sync_gen >= sync_gen)
>  		goto out;
>  
> -	if (__lockref_is_dead(&qd->qd_lockref))
> +	if (lockref_is_dead(&qd->qd_lockref))
>  		goto out;
>  	qd->qd_lockref.count++;
>  
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 21d67af781da..17b9d643e1a8 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -80,7 +80,7 @@ xfs_buf_stale(
>  
>  	spin_lock(&bp->b_lockref.lock);
>  	atomic_set(&bp->b_lru_ref, 0);
> -	if (!__lockref_is_dead(&bp->b_lockref))
> +	if (!lockref_is_dead(&bp->b_lockref))
>  		list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru);
>  	spin_unlock(&bp->b_lockref.lock);
>  }
> @@ -841,7 +841,7 @@ static void
>  xfs_buf_destroy(
>  	struct xfs_buf		*bp)
>  {
> -	ASSERT(__lockref_is_dead(&bp->b_lockref));
> +	ASSERT(lockref_is_dead(&bp->b_lockref));
>  	ASSERT(!(bp->b_flags & _XBF_DELWRI_Q));
>  
>  	if (bp->b_pag)
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 896b24f87ac9..99a82107b8e6 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -128,7 +128,7 @@ xfs_qm_dqpurge(
>  	struct xfs_quotainfo	*qi = dqp->q_mount->m_quotainfo;
>  
>  	spin_lock(&dqp->q_lockref.lock);
> -	if (dqp->q_lockref.count > 0 || __lockref_is_dead(&dqp->q_lockref)) {
> +	if (dqp->q_lockref.count > 0 || lockref_is_dead(&dqp->q_lockref)) {
>  		spin_unlock(&dqp->q_lockref.lock);
>  		return -EAGAIN;
>  	}
> @@ -429,7 +429,7 @@ xfs_qm_dquot_isolate(
>  	 * from the LRU, leave it for the freeing task to complete the freeing
>  	 * process rather than risk it being free from under us here.
>  	 */
> -	if (__lockref_is_dead(&dqp->q_lockref))
> +	if (lockref_is_dead(&dqp->q_lockref))
>  		goto out_miss_unlock;
>  
>  	/*
> diff --git a/include/linux/lockref.h b/include/linux/lockref.h
> index 6ded24cdb4a8..ddfb7d3b8cec 100644
> --- a/include/linux/lockref.h
> +++ b/include/linux/lockref.h
> @@ -34,6 +34,8 @@ struct lockref {
>  	};
>  };
>  
> +#define __LOCKREF_DEAD_VAL	-128
> +
>  /**
>   * lockref_init - Initialize a lockref
>   * @lockref: pointer to lockref structure
> @@ -55,9 +57,15 @@ void lockref_mark_dead(struct lockref *lockref);
>  bool lockref_get_not_dead(struct lockref *lockref);
>  
>  /* Must be called under spinlock for reliable results */
> -static inline bool __lockref_is_dead(const struct lockref *l)
> +static inline bool lockref_is_dead(const struct lockref *l)
> +{
> +	return (READ_ONCE(l->count) == __LOCKREF_DEAD_VAL);
> +}
> +
> +static inline bool lockref_is_dead_or_zero(const struct lockref *l)
>  {
> -	return ((int)l->count < 0);
> +	int count = READ_ONCE(l->count);
> +	return (count == __LOCKREF_DEAD_VAL || count == 0);
>  }
>  
>  #endif /* __LINUX_LOCKREF_H */
> diff --git a/lib/lockref.c b/lib/lockref.c
> index 5d8e3ef3860e..9b3dd688d8cd 100644
> --- a/lib/lockref.c
> +++ b/lib/lockref.c
> @@ -131,7 +131,7 @@ EXPORT_SYMBOL(lockref_put_or_lock);
>  void lockref_mark_dead(struct lockref *lockref)
>  {
>  	assert_spin_locked(&lockref->lock);
> -	lockref->count = -128;
> +	lockref->count = __LOCKREF_DEAD_VAL;
>  }
>  EXPORT_SYMBOL(lockref_mark_dead);
>  
> -- 
> 2.48.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 2/2] dcache: use lockref routines for dead count checks

[Mateusz Guzik]

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/dcache.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 3e9af9de7074..2aee85f3fbaa 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -434,7 +434,7 @@ static inline void __d_clear_type_and_inode(struct dentry *dentry)
 static void dentry_free(struct dentry *dentry)
 {
 	DENTRY_WARN_ONCE(d_really_is_positive(dentry), dentry);
-	DENTRY_WARN_ONCE(dentry->d_lockref.count >= 0, dentry);
+	DENTRY_WARN_ONCE(!lockref_is_dead(&dentry->d_lockref), dentry);
 	D_FLAG_VERIFY(dentry, 0);
 	if (unlikely(dname_external(dentry))) {
 		struct external_name *p = external_name(dentry);
@@ -782,7 +782,7 @@ static bool lock_for_kill(struct dentry *dentry)
  *
  * If @dentry is idle and remains such after we assemble the full
  * locking environment for eviction (see lock_for_kill() for details)
- * we mark it doomed (->d_lockref.count < 0) and proceed to detaching
+ * we mark it doomed (see lockref_mark_dead()) and proceed to detaching
  * it from any filesystem objects.  Otherwise we drop ->d_lock and
  * return %NULL.
  *
@@ -946,7 +946,7 @@ static inline bool fast_dput(struct dentry *dentry)
 	if (unlikely(ret < 0)) {
 		spin_lock(&dentry->d_lock);
 		rcu_read_unlock();
-		if (WARN_ON_ONCE(dentry->d_lockref.count <= 0)) {
+		if (WARN_ON_ONCE(lockref_is_dead_or_zero(&dentry->d_lockref))) {
 			spin_unlock(&dentry->d_lock);
 			return true;
 		}
@@ -1644,7 +1644,7 @@ static enum d_walk_ret select_collect(void *_data, struct dentry *dentry)
 	if (data->start == dentry)
 		goto out;
 
-	if (dentry->d_lockref.count <= 0) {
+	if (lockref_is_dead_or_zero(&dentry->d_lockref)) {
 		__move_to_shrink_list(dentry, &data->dispose);
 		data->found++;
 	}
@@ -1676,7 +1676,7 @@ static enum d_walk_ret select_collect2(void *_data, struct dentry *dentry)
 	if (data->start == dentry)
 		goto out;
 
-	if (dentry->d_lockref.count <= 0) {
+	if (lockref_is_dead_or_zero(&dentry->d_lockref)) {
 		if (!__move_to_shrink_list(dentry, &data->dispose)) {
 			/*
 			 * We need an enter RCU read-side critical area that
@@ -1747,7 +1747,7 @@ static void shrink_dcache_tree(struct dentry *parent, bool for_umount)
 			spin_lock(&v->d_lock);
 			rcu_read_unlock();
 
-			if (unlikely(v->d_lockref.count < 0)) {
+			if (unlikely(lockref_is_dead(&v->d_lockref))) {
 				// It's doomed; if it isn't dead yet, notify us
 				// once it becomes invisible to d_walk().
 				need_wait = d_add_waiter(v, &wait);
@@ -1823,7 +1823,7 @@ void shrink_dcache_for_umount(struct super_block *sb)
 		spin_unlock(&sb->s_roots_lock);
 		spin_lock(&dentry->d_lock);
 		rcu_read_unlock();
-		if (unlikely(dentry->d_lockref.count < 0)) {
+		if (unlikely(lockref_is_dead(&dentry->d_lockref))) {
 			struct completion_list wait;
 			bool need_wait = d_add_waiter(dentry, &wait);
 
@@ -2822,7 +2822,7 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
 		spin_lock(&dentry->d_lock);
 		rcu_read_unlock();
 		/* now we can try to grab a reference */
-		if (unlikely(dentry->d_lockref.count < 0)) {
+		if (unlikely(lockref_is_dead(&dentry->d_lockref))) {
 			spin_unlock(&dentry->d_lock);
 			goto retry;
 		}
-- 
2.48.1

Re: [PATCH 2/2] dcache: use lockref routines for dead count checks

[Jan Kara]

On Fri 24-07-26 19:14:22, Mateusz Guzik wrote:
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/dcache.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 3e9af9de7074..2aee85f3fbaa 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -434,7 +434,7 @@ static inline void __d_clear_type_and_inode(struct dentry *dentry)
>  static void dentry_free(struct dentry *dentry)
>  {
>  	DENTRY_WARN_ONCE(d_really_is_positive(dentry), dentry);
> -	DENTRY_WARN_ONCE(dentry->d_lockref.count >= 0, dentry);
> +	DENTRY_WARN_ONCE(!lockref_is_dead(&dentry->d_lockref), dentry);
>  	D_FLAG_VERIFY(dentry, 0);
>  	if (unlikely(dname_external(dentry))) {
>  		struct external_name *p = external_name(dentry);
> @@ -782,7 +782,7 @@ static bool lock_for_kill(struct dentry *dentry)
>   *
>   * If @dentry is idle and remains such after we assemble the full
>   * locking environment for eviction (see lock_for_kill() for details)
> - * we mark it doomed (->d_lockref.count < 0) and proceed to detaching
> + * we mark it doomed (see lockref_mark_dead()) and proceed to detaching
>   * it from any filesystem objects.  Otherwise we drop ->d_lock and
>   * return %NULL.
>   *
> @@ -946,7 +946,7 @@ static inline bool fast_dput(struct dentry *dentry)
>  	if (unlikely(ret < 0)) {
>  		spin_lock(&dentry->d_lock);
>  		rcu_read_unlock();
> -		if (WARN_ON_ONCE(dentry->d_lockref.count <= 0)) {
> +		if (WARN_ON_ONCE(lockref_is_dead_or_zero(&dentry->d_lockref))) {
>  			spin_unlock(&dentry->d_lock);
>  			return true;
>  		}
> @@ -1644,7 +1644,7 @@ static enum d_walk_ret select_collect(void *_data, struct dentry *dentry)
>  	if (data->start == dentry)
>  		goto out;
>  
> -	if (dentry->d_lockref.count <= 0) {
> +	if (lockref_is_dead_or_zero(&dentry->d_lockref)) {
>  		__move_to_shrink_list(dentry, &data->dispose);
>  		data->found++;
>  	}
> @@ -1676,7 +1676,7 @@ static enum d_walk_ret select_collect2(void *_data, struct dentry *dentry)
>  	if (data->start == dentry)
>  		goto out;
>  
> -	if (dentry->d_lockref.count <= 0) {
> +	if (lockref_is_dead_or_zero(&dentry->d_lockref)) {
>  		if (!__move_to_shrink_list(dentry, &data->dispose)) {
>  			/*
>  			 * We need an enter RCU read-side critical area that
> @@ -1747,7 +1747,7 @@ static void shrink_dcache_tree(struct dentry *parent, bool for_umount)
>  			spin_lock(&v->d_lock);
>  			rcu_read_unlock();
>  
> -			if (unlikely(v->d_lockref.count < 0)) {
> +			if (unlikely(lockref_is_dead(&v->d_lockref))) {
>  				// It's doomed; if it isn't dead yet, notify us
>  				// once it becomes invisible to d_walk().
>  				need_wait = d_add_waiter(v, &wait);
> @@ -1823,7 +1823,7 @@ void shrink_dcache_for_umount(struct super_block *sb)
>  		spin_unlock(&sb->s_roots_lock);
>  		spin_lock(&dentry->d_lock);
>  		rcu_read_unlock();
> -		if (unlikely(dentry->d_lockref.count < 0)) {
> +		if (unlikely(lockref_is_dead(&dentry->d_lockref))) {
>  			struct completion_list wait;
>  			bool need_wait = d_add_waiter(dentry, &wait);
>  
> @@ -2822,7 +2822,7 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
>  		spin_lock(&dentry->d_lock);
>  		rcu_read_unlock();
>  		/* now we can try to grab a reference */
> -		if (unlikely(dentry->d_lockref.count < 0)) {
> +		if (unlikely(lockref_is_dead(&dentry->d_lockref))) {
>  			spin_unlock(&dentry->d_lock);
>  			goto retry;
>  		}
> -- 
> 2.48.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/2] lockref tidy ups + touch ups to it's usage by dcache

[Christian Brauner]

On Fri, 24 Jul 2026 19:14:20 +0200, Mateusz Guzik wrote:
> lockref tidy ups + touch ups to it's usage by dcache
> 
> The open-coded check for < 0 indicating a dead object uses information
> it should not know, so to speak.
> 
> This is a preliminary clean up for a longer term goal of moving dcache
> away from lockref into an approach which can atomic_add both ways
> instead of suffering the cmpxchg loop.
> 
> [...]

Nice cleanup!

---

Applied to the vfs-7.3.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.3.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.3.misc

[1/2] lockref: tidy up dead count handling
      https://git.kernel.org/vfs/vfs/c/21a8e8068278
[2/2] dcache: use lockref routines for dead count checks
      https://git.kernel.org/vfs/vfs/c/4f87acca1a2e