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

From: Chandan Babu R <chandanrlinux@gmail.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: sandeen@sandeen.net, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 4/4] xfs_scrub: handle concurrent directory updates during name scan
Date: Mon, 18 Jan 2021 10:17:50 +0530	[thread overview]
Message-ID: <87h7nen955.fsf@garuda> (raw)
In-Reply-To: <161076033640.3386689.11320451390779411930.stgit@magnolia>

On 16 Jan 2021 at 06:55, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> 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.
>

Looks good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  scrub/unicrash.c |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
>
> 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) &&

-- 
chandan