[PATCH v1 08/12] libfs: replace find_positive_dentry() with scan_positives()

From: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>

scan_positives() can be used in place of find_positive_dentry().

This makes offset_iterate_dir() simpler as 'next' is no longer needed.

offset_dir_lookup() needs an extra loop to find a child on which a ref
can be taken, as scan_positives() requires a dentry with a ref.  I think
this makes a cleaner separation as the RCU read lock no longer needs to
be held over the scan call.  This will be important in a future patch.

Using scan_positives() means, after the next patch, that cursors can
appear on the d_sib list, so when finding a dentry from the offset,
which might be dead by now (dentry_kill()) we need to step over any
cursors to find something we can take a reference on.  This requires
parent->d_lock as cursors are not RCU-safe.

We add lockref_get_not_dead_nested() so that we can opportunistically
get a ref on a child while holding the lock on the parent.

Signed-off-by: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
---
 fs/libfs.c              | 58 ++++++++++++++++-------------------------
 include/linux/lockref.h | 12 ++++++++-
 lib/lockref.c           | 10 ++++---
 3 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index 260f56863742..c0771e8b645c 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -435,47 +435,39 @@ static loff_t offset_dir_llseek(struct file *file, loff_t offset, int whence)
 	return vfs_setpos(file, offset, LONG_MAX);
 }
 
-static struct dentry *find_positive_dentry(struct dentry *parent,
-					   struct dentry *dentry,
-					   bool next)
-{
-	struct dentry *found = NULL;
-
-	spin_lock(&parent->d_lock);
-	if (next)
-		dentry = d_next_sibling(dentry);
-	else if (!dentry)
-		dentry = d_first_child(parent);
-	hlist_for_each_entry_from(dentry, d_sib) {
-		if (!simple_positive(dentry))
-			continue;
-		spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
-		if (simple_positive(dentry))
-			found = dget_dlock(dentry);
-		spin_unlock(&dentry->d_lock);
-		if (likely(found))
-			break;
-	}
-	spin_unlock(&parent->d_lock);
-	return found;
-}
-
 static noinline_for_stack struct dentry *
 offset_dir_lookup(struct dentry *parent, loff_t offset)
 {
 	struct inode *inode = d_inode(parent);
 	struct offset_ctx *octx = inode->i_op->get_offset_ctx(inode);
-	struct dentry *child, *found = NULL;
+	struct dentry *found = NULL;
 
 	MA_STATE(mas, &octx->mt, offset, offset);
 
 	if (offset == DIR_OFFSET_FIRST)
-		found = find_positive_dentry(parent, NULL, false);
+		found = scan_positives(parent, NULL, NULL, 1);
 	else {
 		rcu_read_lock();
-		child = mas_find_rev(&mas, DIR_OFFSET_MIN);
-		found = find_positive_dentry(parent, child, false);
+		spin_lock(&parent->d_lock);
+		found = mas_find_rev(&mas, DIR_OFFSET_MIN);
+		if (found) {
+			/*
+			 * parent lock ensures found is still on
+			 * the d_sib list, but it could be have been marked dead.
+			 * So we need to find something we can get a ref on,
+			 * which isn't a cursor.
+			 */
+			hlist_for_each_entry_from(found, d_sib) {
+				if (!(found->d_flags & DCACHE_DENTRY_CURSOR) &&
+				    lockref_get_not_dead_nested(&found->d_lockref,
+								DENTRY_D_LOCK_NESTED))
+					break;
+			}
+		}
+		spin_unlock(&parent->d_lock);
 		rcu_read_unlock();
+		if (found && !simple_positive(found))
+			found = scan_positives(parent, NULL, found, 1);
 	}
 	return found;
 }
@@ -497,18 +489,14 @@ static void offset_iterate_dir(struct file *file, struct dir_context *ctx)
 	if (!dentry)
 		goto out_eod;
 	while (true) {
-		struct dentry *next;
-
 		ctx->pos = dentry2offset(dentry);
 		if (!offset_dir_emit(ctx, dentry))
 			break;
 
-		next = find_positive_dentry(dir, dentry, true);
-		dput(dentry);
+		dentry = scan_positives(dir, NULL, dentry, 1);
 
-		if (!next)
+		if (!dentry)
 			goto out_eod;
-		dentry = next;
 	}
 	dput(dentry);
 	return;
diff --git a/include/linux/lockref.h b/include/linux/lockref.h
index 6ded24cdb4a8..9da4fbc2cd13 100644
--- a/include/linux/lockref.h
+++ b/include/linux/lockref.h
@@ -52,7 +52,17 @@ bool lockref_get_not_zero(struct lockref *lockref);
 bool lockref_put_or_lock(struct lockref *lockref) __cond_acquires(false, &lockref->lock);
 
 void lockref_mark_dead(struct lockref *lockref);
-bool lockref_get_not_dead(struct lockref *lockref);
+bool lockref_get_not_dead_nested(struct lockref *lockref, int subclass);
+/**
+ * lockref_get_not_dead - Increments count unless the ref is dead
+ * @lockref: pointer to lockref structure
+ *
+ * Return: 1 if count updated successfully or 0 if lockref was dead
+ */
+static inline bool lockref_get_not_dead(struct lockref *lockref)
+{
+	return lockref_get_not_dead_nested(lockref, 0);
+}
 
 /* Must be called under spinlock for reliable results */
 static inline bool __lockref_is_dead(const struct lockref *l)
diff --git a/lib/lockref.c b/lib/lockref.c
index 5d8e3ef3860e..dddd26100a51 100644
--- a/lib/lockref.c
+++ b/lib/lockref.c
@@ -136,11 +136,13 @@ void lockref_mark_dead(struct lockref *lockref)
 EXPORT_SYMBOL(lockref_mark_dead);
 
 /**
- * lockref_get_not_dead - Increments count unless the ref is dead
+ * lockref_get_not_dead_nested - Increments count unless the ref is dead
  * @lockref: pointer to lockref structure
+ * @subclass: lockdep class for taking ->lock
+ *
  * Return: 1 if count updated successfully or 0 if lockref was dead
  */
-bool lockref_get_not_dead(struct lockref *lockref)
+bool lockref_get_not_dead_nested(struct lockref *lockref, int subclass)
 {
 	bool retval = false;
 
@@ -152,7 +154,7 @@ bool lockref_get_not_dead(struct lockref *lockref)
 		return true;
 	);
 
-	spin_lock(&lockref->lock);
+	spin_lock_nested(&lockref->lock, subclass);
 	if (lockref->count >= 0) {
 		lockref->count++;
 		retval = true;
@@ -160,4 +162,4 @@ bool lockref_get_not_dead(struct lockref *lockref)
 	spin_unlock(&lockref->lock);
 	return retval;
 }
-EXPORT_SYMBOL(lockref_get_not_dead);
+EXPORT_SYMBOL(lockref_get_not_dead_nested);
-- 
2.50.0.107.gf914562f5916.dirty