[v3] Add more tests for multi fs block atomic writes

[PATCH v3 02/13] common/rc: Fix fsx for ext4 with bigalloc

Posted by Ojaswin Mujoo 2 months, 3 weeks ago

Insert range and collapse range only works with bigalloc in case
the range is cluster size aligned, which fsx doesnt take care. To
work past this, disable insert range and collapse range on ext4, if
bigalloc is enabled.

This is achieved by defining a new function _set_default_fsx_avoid
called via run_fsx helper. This can be used to selectively disable
fsx options based on the configuration.

Co-developed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 common/rc | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/common/rc b/common/rc
index 9a9d3cc8..218cf253 100644
--- a/common/rc
+++ b/common/rc
@@ -5113,10 +5113,37 @@ _require_hugepage_fsx()
 		_notrun "fsx binary does not support MADV_COLLAPSE"
 }
 
+_set_default_fsx_avoid() {
+	local file=$1
+
+	case "$FSTYP" in
+	"ext4")
+		local dev=$(findmnt -n -o SOURCE --target $file)
+
+		# open code instead of _require_dumpe2fs cause we don't
+		# want to _notrun if dumpe2fs is not available
+		if [ -z "$DUMPE2FS_PROG" ]; then
+			echo "_set_default_fsx_avoid: dumpe2fs not found, skipping bigalloc check." >> $seqres.full
+			return
+		fi
+
+		$DUMPE2FS_PROG -h $dev 2>&1 | grep -q bigalloc && {
+			export FSX_AVOID+=" -I -C"
+		}
+		;;
+	# Add other filesystem types here as needed
+	*)
+		;;
+	esac
+}
+
 _run_fsx()
 {
 	echo "fsx $*"
 	local args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
+
+	_set_default_fsx_avoid $testfile
+
 	set -- $FSX_PROG $args $FSX_AVOID $TEST_DIR/junk
 	echo "$@" >>$seqres.full
 	rm -f $TEST_DIR/junk
-- 
2.49.0

Re: [PATCH v3 02/13] common/rc: Fix fsx for ext4 with bigalloc

Posted by Darrick J. Wong 2 months, 3 weeks ago

On Sat, Jul 12, 2025 at 07:42:44PM +0530, Ojaswin Mujoo wrote:
> Insert range and collapse range only works with bigalloc in case
> the range is cluster size aligned, which fsx doesnt take care. To
> work past this, disable insert range and collapse range on ext4, if
> bigalloc is enabled.
> 
> This is achieved by defining a new function _set_default_fsx_avoid
> called via run_fsx helper. This can be used to selectively disable
> fsx options based on the configuration.
> 
> Co-developed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> ---
>  common/rc | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 9a9d3cc8..218cf253 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5113,10 +5113,37 @@ _require_hugepage_fsx()
>  		_notrun "fsx binary does not support MADV_COLLAPSE"
>  }
>  
> +_set_default_fsx_avoid() {
> +	local file=$1
> +
> +	case "$FSTYP" in
> +	"ext4")
> +		local dev=$(findmnt -n -o SOURCE --target $file)
> +
> +		# open code instead of _require_dumpe2fs cause we don't
> +		# want to _notrun if dumpe2fs is not available
> +		if [ -z "$DUMPE2FS_PROG" ]; then
> +			echo "_set_default_fsx_avoid: dumpe2fs not found, skipping bigalloc check." >> $seqres.full
> +			return
> +		fi

I hate to be the guy who says one thing and then another, but ...

If we extended _get_file_block_size to report the ext4 bigalloc cluster
size, would that be sufficient to keep testing collapse/insert range?

I guess the tricky part here is that bigalloc allows sub-cluster
mappings and we might not want to do all file IO testing in such big
units.

> +
> +		$DUMPE2FS_PROG -h $dev 2>&1 | grep -q bigalloc && {
> +			export FSX_AVOID+=" -I -C"

No need to export FSX_AVOID to subprocesses.

--D

> +		}
> +		;;
> +	# Add other filesystem types here as needed
> +	*)
> +		;;
> +	esac
> +}
> +
>  _run_fsx()
>  {
>  	echo "fsx $*"
>  	local args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
> +
> +	_set_default_fsx_avoid $testfile
> +
>  	set -- $FSX_PROG $args $FSX_AVOID $TEST_DIR/junk
>  	echo "$@" >>$seqres.full
>  	rm -f $TEST_DIR/junk
> -- 
> 2.49.0
> 
>

Re: [PATCH v3 02/13] common/rc: Fix fsx for ext4 with bigalloc

Posted by Ojaswin Mujoo 2 months, 2 weeks ago

On Thu, Jul 17, 2025 at 09:11:54AM -0700, Darrick J. Wong wrote:
> On Sat, Jul 12, 2025 at 07:42:44PM +0530, Ojaswin Mujoo wrote:
> > Insert range and collapse range only works with bigalloc in case
> > the range is cluster size aligned, which fsx doesnt take care. To
> > work past this, disable insert range and collapse range on ext4, if
> > bigalloc is enabled.
> > 
> > This is achieved by defining a new function _set_default_fsx_avoid
> > called via run_fsx helper. This can be used to selectively disable
> > fsx options based on the configuration.
> > 
> > Co-developed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > ---
> >  common/rc | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index 9a9d3cc8..218cf253 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -5113,10 +5113,37 @@ _require_hugepage_fsx()
> >  		_notrun "fsx binary does not support MADV_COLLAPSE"
> >  }
> >  
> > +_set_default_fsx_avoid() {
> > +	local file=$1
> > +
> > +	case "$FSTYP" in
> > +	"ext4")
> > +		local dev=$(findmnt -n -o SOURCE --target $file)
> > +
> > +		# open code instead of _require_dumpe2fs cause we don't
> > +		# want to _notrun if dumpe2fs is not available
> > +		if [ -z "$DUMPE2FS_PROG" ]; then
> > +			echo "_set_default_fsx_avoid: dumpe2fs not found, skipping bigalloc check." >> $seqres.full
> > +			return
> > +		fi
> 
> I hate to be the guy who says one thing and then another, but ...
> 
> If we extended _get_file_block_size to report the ext4 bigalloc cluster
> size, would that be sufficient to keep testing collapse/insert range?
> 
> I guess the tricky part here is that bigalloc allows sub-cluster
> mappings and we might not want to do all file IO testing in such big
> units.

Hmm, so maybe a better way is to just add a parameter like alloc_unit in
fsx where we can pass the cluster_size to which INSERT/COLLAPSE range be
aligned to. For now we can pass it explicitly in the tests if needed.

I do plan on working on your suggestion of exposing alloc unit via
statx(). Once we have that in the kernel, fsx can use that as well.

If this approach sounds okay I can try to maybe send the whole "fixing
of insert/collpase range in fsx" as a patchset separate from atomic
writes.


> 
> > +
> > +		$DUMPE2FS_PROG -h $dev 2>&1 | grep -q bigalloc && {
> > +			export FSX_AVOID+=" -I -C"
> 
> No need to export FSX_AVOID to subprocesses.
> 
> --D

Got it, will fix. Thanks for review!


Regards,
ojaswin
> 
> > +		}
> > +		;;
> > +	# Add other filesystem types here as needed
> > +	*)
> > +		;;
> > +	esac
> > +}
> > +
> >  _run_fsx()
> >  {
> >  	echo "fsx $*"
> >  	local args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
> > +
> > +	_set_default_fsx_avoid $testfile
> > +
> >  	set -- $FSX_PROG $args $FSX_AVOID $TEST_DIR/junk
> >  	echo "$@" >>$seqres.full
> >  	rm -f $TEST_DIR/junk
> > -- 
> > 2.49.0
> > 
> >

Re: [PATCH v3 02/13] common/rc: Fix fsx for ext4 with bigalloc

Posted by Darrick J. Wong 2 months, 2 weeks ago

On Tue, Jul 22, 2025 at 03:23:02PM +0530, Ojaswin Mujoo wrote:
> On Thu, Jul 17, 2025 at 09:11:54AM -0700, Darrick J. Wong wrote:
> > On Sat, Jul 12, 2025 at 07:42:44PM +0530, Ojaswin Mujoo wrote:
> > > Insert range and collapse range only works with bigalloc in case
> > > the range is cluster size aligned, which fsx doesnt take care. To
> > > work past this, disable insert range and collapse range on ext4, if
> > > bigalloc is enabled.
> > > 
> > > This is achieved by defining a new function _set_default_fsx_avoid
> > > called via run_fsx helper. This can be used to selectively disable
> > > fsx options based on the configuration.
> > > 
> > > Co-developed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > > ---
> > >  common/rc | 27 +++++++++++++++++++++++++++
> > >  1 file changed, 27 insertions(+)
> > > 
> > > diff --git a/common/rc b/common/rc
> > > index 9a9d3cc8..218cf253 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -5113,10 +5113,37 @@ _require_hugepage_fsx()
> > >  		_notrun "fsx binary does not support MADV_COLLAPSE"
> > >  }
> > >  
> > > +_set_default_fsx_avoid() {
> > > +	local file=$1
> > > +
> > > +	case "$FSTYP" in
> > > +	"ext4")
> > > +		local dev=$(findmnt -n -o SOURCE --target $file)
> > > +
> > > +		# open code instead of _require_dumpe2fs cause we don't
> > > +		# want to _notrun if dumpe2fs is not available
> > > +		if [ -z "$DUMPE2FS_PROG" ]; then
> > > +			echo "_set_default_fsx_avoid: dumpe2fs not found, skipping bigalloc check." >> $seqres.full
> > > +			return
> > > +		fi
> > 
> > I hate to be the guy who says one thing and then another, but ...
> > 
> > If we extended _get_file_block_size to report the ext4 bigalloc cluster
> > size, would that be sufficient to keep testing collapse/insert range?
> > 
> > I guess the tricky part here is that bigalloc allows sub-cluster
> > mappings and we might not want to do all file IO testing in such big
> > units.
> 
> Hmm, so maybe a better way is to just add a parameter like alloc_unit in
> fsx where we can pass the cluster_size to which INSERT/COLLAPSE range be
> aligned to. For now we can pass it explicitly in the tests if needed.
> 
> I do plan on working on your suggestion of exposing alloc unit via
> statx(). Once we have that in the kernel, fsx can use that as well.
> 
> If this approach sounds okay I can try to maybe send the whole "fixing
> of insert/collpase range in fsx" as a patchset separate from atomic
> writes.

Yeah, that sounds like a good longer-term solution to me. :)

--D

> > 
> > > +
> > > +		$DUMPE2FS_PROG -h $dev 2>&1 | grep -q bigalloc && {
> > > +			export FSX_AVOID+=" -I -C"
> > 
> > No need to export FSX_AVOID to subprocesses.
> > 
> > --D
> 
> Got it, will fix. Thanks for review!
> 
> 
> Regards,
> ojaswin
> > 
> > > +		}
> > > +		;;
> > > +	# Add other filesystem types here as needed
> > > +	*)
> > > +		;;
> > > +	esac
> > > +}
> > > +
> > >  _run_fsx()
> > >  {
> > >  	echo "fsx $*"
> > >  	local args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
> > > +
> > > +	_set_default_fsx_avoid $testfile
> > > +
> > >  	set -- $FSX_PROG $args $FSX_AVOID $TEST_DIR/junk
> > >  	echo "$@" >>$seqres.full
> > >  	rm -f $TEST_DIR/junk
> > > -- 
> > > 2.49.0
> > > 
> > > 
>