[4/6] xfs_scrub: handle concurrent directory updates during name scan

Commit Message

Darrick J. Wong Feb. 9, 2021, 4:11 a.m. UTC

From: Darrick J. Wong <[e-mail]>

The name scanner in xfs_scrub cannot lock a namespace (dirent or xattr)
and the kernel does not provide a stable cursor interface, which means
that we can see the same byte sequence multiple times during a scan.
This isn't a confusing name error since the kernel enforces uniqueness
on the byte sequence, so all we need to do here is update the old entry.

Signed-off-by: Darrick J. Wong <[e-mail]>
Reviewed-by: Chandan Babu R <[e-mail]>
---
 scrub/unicrash.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig Feb. 9, 2021, 9:30 a.m. UTC | #1

On Mon, Feb 08, 2021 at 08:11:38PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <[e-mail]>
> 
> The name scanner in xfs_scrub cannot lock a namespace (dirent or xattr)
> and the kernel does not provide a stable cursor interface, which means
> that we can see the same byte sequence multiple times during a scan.
> This isn't a confusing name error since the kernel enforces uniqueness
> on the byte sequence, so all we need to do here is update the old entry.

So we get the same name but a different ino?  I guess that can happen
with a replacing rename.  Maybe state that more clearly?

Otherwise looks good:

Reviewed-by: Christoph Hellwig <[e-mail]>

Darrick J. Wong Feb. 9, 2021, 4:53 p.m. UTC | #2

On Tue, Feb 09, 2021 at 09:30:32AM +0000, Christoph Hellwig wrote:
> On Mon, Feb 08, 2021 at 08:11:38PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <[e-mail]>
> > 
> > The name scanner in xfs_scrub cannot lock a namespace (dirent or xattr)
> > and the kernel does not provide a stable cursor interface, which means
> > that we can see the same byte sequence multiple times during a scan.
> > This isn't a confusing name error since the kernel enforces uniqueness
> > on the byte sequence, so all we need to do here is update the old entry.
> 
> So we get the same name but a different ino?  I guess that can happen
> with a replacing rename.  Maybe state that more clearly?

Ok, the paragraph now reads:

"The name scanner in xfs_scrub cannot lock a namespace (dirent or xattr)
and the kernel does not provide a stable cursor interface, which means
that we can see the same byte sequence multiple times during a scan if
other processes are performing replacing renames on the directory
simultaneously.  This isn't a confusing name error since the kernel
enforces uniqueness on the byte sequence, so all we need to do here is
update the old entry."

--D

> 
> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <[e-mail]>

Patch

diff --git a/scrub/unicrash.c b/scrub/unicrash.c
index de3217c2..cb0880c1 100644
--- a/scrub/unicrash.c
+++ b/scrub/unicrash.c
@@ -68,7 +68,7 @@  struct name_entry {
 
 	xfs_ino_t		ino;
 
-	/* Raw UTF8 name */
+	/* Raw dirent name */
 	size_t			namelen;
 	char			name[0];
 };
@@ -627,6 +627,20 @@  unicrash_add(
 	uc->buckets[bucket] = new_entry;
 
 	while (entry != NULL) {
+		/*
+		 * If we see the same byte sequence then someone's modifying
+		 * the namespace while we're scanning it.  Update the existing
+		 * entry's inode mapping and erase the new entry from existence.
+		 */
+		if (new_entry->namelen == entry->namelen &&
+		    !memcmp(new_entry->name, entry->name, entry->namelen)) {
+			entry->ino = new_entry->ino;
+			uc->buckets[bucket] = new_entry->next;
+			name_entry_free(new_entry);
+			*badflags = 0;
+			return;
+		}
+
 		/* Same normalization? */
 		if (new_entry->normstrlen == entry->normstrlen &&
 		    !u_strcmp(new_entry->normstr, entry->normstr) &&