[PATCH v2 11/18] libfs: replace find_positive_dentry() with scan_positives()

NeilBrown <[email protected]>
Newsgroups org.kernel.vger.linux-fsdevel,org.kernel.vger.autofs,org.kernel.vger.ceph-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-nfs
Message-ID <[email protected]>
From: NeilBrown <[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 <[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 21abe0074c77..44e8dec51237 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -438,47 +438,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, and isn't anything NORCU.
+			 */
+			hlist_for_each_entry_from(found, d_sib) {
+				if (!(found->d_flags & DCACHE_NORCU) &&
+				    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;
 }
@@ -500,18 +492,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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.