FS/XFS testing framework
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Dave Chinner <david@fromorbit.com>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH 2/3] populate: ensure btree directories are created reliably
Date: Tue, 10 Jan 2023 21:47:26 -0800	[thread overview]
Message-ID: <Y75NbhbmZJMnVBFK@magnolia> (raw)
In-Reply-To: <20230110224906.1171483-3-david@fromorbit.com>

On Wed, Jan 11, 2023 at 09:49:05AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The population function creates an XFS btree format directory by
> polling the extent count of the inode and creating new dirents until
> the extent count goes over the limit that pushes it into btree
> format.
> 
> It then removes every second dirent to create empty space in the
> directory data to ensure that operations like metadump with
> obfuscation can check that they don't leak stale data from deleted
> dirents.
> 
> Whilst this does not result in directory data blocks being freed, it
> does not take into account the fact that the dabtree index has half
> the entries removed from it and that can result in btree nodes
> merging and extents being freed. This causes the extent count to go
> down, and the inode is converted back into extent form. The
> population checks then fail because it should be in btree form.
> 
> Fix this by counting the number of directory data extents rather than
> the total number of extents in the data fork. We can do this simply
> by using xfs_bmap and counting the number of extents returned as it
> does not report extents beyond EOF (which is where the dabtree is
> located). As the number of data blocks does not change with the
> dirent removal algorithm used, this will ensure that the inode data
> fork remains in btree format.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  common/populate | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/common/populate b/common/populate
> index 9b60fa5c1..7b5b16fb8 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -80,8 +80,11 @@ __populate_create_nfiles() {
>  			continue
>  		fi
>  
> -		local nextents="$(_xfs_get_fsxattr nextents $name)"
> -		if [ "${nextents}" -gt "${max_nextents}" ]; then
> +		# Extent count checks use data blocks only to avoid the removal
> +		# step from removing dabtree index blocks and reducing the
> +		# number of extents below the required threshold.
> +		local nextents="$(xfs_bmap ${name} |grep -v hole | wc -l)"
> +		if [ "$((nextents - 1))" -gt "${max_nextents}" ]; then

Pretty much the same patch I had in my tree...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  			echo ${d}
>  			break
>  		fi
> -- 
> 2.38.1
> 

  reply	other threads:[~2023-01-11  5:47 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-10 22:49 [PATCH 0/3] fstests: filesystem population fixes Dave Chinner
2023-01-10 22:49 ` [PATCH 1/3] populate: fix horrible performance due to excessive forking Dave Chinner
2023-01-11  6:02   ` Darrick J. Wong
2023-01-12  1:58     ` Darrick J. Wong
2023-01-12 10:24       ` [PATCH 1/3] more python dependence. was: " David Disseldorp
2023-01-12 17:07         ` Darrick J. Wong
2023-01-12 20:23           ` David Disseldorp
2023-01-12 20:42           ` Zorro Lang
2023-01-15 18:33             ` Darrick J. Wong
2023-01-10 22:49 ` [PATCH 2/3] populate: ensure btree directories are created reliably Dave Chinner
2023-01-11  5:47   ` Darrick J. Wong [this message]
2023-01-12  5:42   ` Gao Xiang
2023-01-10 22:49 ` [PATCH 3/3] xfs/294: performance is unreasonably slow Dave Chinner
2023-01-11 20:29   ` David Disseldorp
2023-01-12  8:39   ` Zorro Lang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y75NbhbmZJMnVBFK@magnolia \
    --to=djwong@kernel.org \
    --cc=david@fromorbit.com \
    --cc=fstests@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox