[PATCH] common/atomicwrites: fix _require_scratch_write_atomic when awu_max_fs is empty

[PATCH] common/atomicwrites: fix _require_scratch_write_atomic when awu_max_fs is empty

[Zorro Lang]

If a kernel doesn't support atomic write, _get_atomic_write_unit_min
and _get_atomic_write_unit_max will get nothing.

  local awu_min_bdev=$(_get_atomic_write_unit_min $SCRATCH_DEV)
  local awu_max_bdev=$(_get_atomic_write_unit_max $SCRATCH_DEV)

If $awu_min_bdev and $awu_max_bdev are empty, then the logic likes
"[ $awu_min_bdev -eq 0 ] && [ $awu_max_bdev -eq 0 ]" is meaningless,
and it causes g/767 hang on system which doesn't support atomic
write.

Signed-off-by: Zorro Lang <zlang@kernel.org>
---
 common/atomicwrites | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/common/atomicwrites b/common/atomicwrites
index 33526399d..bbcc4e7c5 100644
--- a/common/atomicwrites
+++ b/common/atomicwrites
@@ -47,8 +47,9 @@ _require_scratch_write_atomic_multi_fsblock()
 
 	_scratch_unmount
 
-	test $awu_max_fs -ge $((bsize * 2)) || \
+	if [ -z "$awu_max_fs" -o $awu_max_fs -lt $((bsize * 2)) ];then
 		_notrun "multi-block atomic writes not supported by this filesystem"
+	fi
 }
 
 _require_scratch_write_atomic()
@@ -58,7 +59,8 @@ _require_scratch_write_atomic()
 	local awu_min_bdev=$(_get_atomic_write_unit_min $SCRATCH_DEV)
 	local awu_max_bdev=$(_get_atomic_write_unit_max $SCRATCH_DEV)
 
-	if [ $awu_min_bdev -eq 0 ] && [ $awu_max_bdev -eq 0 ]; then
+	if [ -z "$awu_min_bdev" -o -z "$awu_max_bdev" ] || \
+		[ $awu_min_bdev -eq 0 -a $awu_max_bdev -eq 0 ];then
 		_notrun "write atomic not supported by this block device"
 	fi
 
@@ -75,7 +77,8 @@ _require_scratch_write_atomic()
 
 	_scratch_unmount
 
-	if [ $awu_min_fs -eq 0 ] && [ $awu_max_fs -eq 0 ]; then
+	if [ -z "$awu_min_fs" -o -z "$awu_max_fs" ] || \
+		[ $awu_min_fs -eq 0 -a $awu_max_fs -eq 0 ];then
 		_notrun "write atomic not supported by this filesystem"
 	fi
 }
-- 
2.49.0

Re: [PATCH] common/atomicwrites: fix _require_scratch_write_atomic when awu_max_fs is empty

[Pavel Reichl]

On 06/09/2025 15:17, Zorro Lang wrote:
> If a kernel doesn't support atomic write, _get_atomic_write_unit_min
> and _get_atomic_write_unit_max will get nothing.
>
>    local awu_min_bdev=$(_get_atomic_write_unit_min $SCRATCH_DEV)
>    local awu_max_bdev=$(_get_atomic_write_unit_max $SCRATCH_DEV)
>
> If $awu_min_bdev and $awu_max_bdev are empty, then the logic likes
> "[ $awu_min_bdev -eq 0 ] && [ $awu_max_bdev -eq 0 ]" is meaningless,
> and it causes g/767 hang on system which doesn't support atomic
> write.
>
> Signed-off-by: Zorro Lang <zlang@kernel.org>
> ---
>   common/atomicwrites | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/common/atomicwrites b/common/atomicwrites
> index 33526399d..bbcc4e7c5 100644
> --- a/common/atomicwrites
> +++ b/common/atomicwrites
> @@ -47,8 +47,9 @@ _require_scratch_write_atomic_multi_fsblock()
>   
>   	_scratch_unmount
>   
> -	test $awu_max_fs -ge $((bsize * 2)) || \
> +	if [ -z "$awu_max_fs" -o $awu_max_fs -lt $((bsize * 2)) ];then
>   		_notrun "multi-block atomic writes not supported by this filesystem"
> +	fi
>   }
>   
>   _require_scratch_write_atomic()
> @@ -58,7 +59,8 @@ _require_scratch_write_atomic()
>   	local awu_min_bdev=$(_get_atomic_write_unit_min $SCRATCH_DEV)
>   	local awu_max_bdev=$(_get_atomic_write_unit_max $SCRATCH_DEV)
>   
> -	if [ $awu_min_bdev -eq 0 ] && [ $awu_max_bdev -eq 0 ]; then
> +	if [ -z "$awu_min_bdev" -o -z "$awu_max_bdev" ] || \
> +		[ $awu_min_bdev -eq 0 -a $awu_max_bdev -eq 0 ];then
>   		_notrun "write atomic not supported by this block device"
>   	fi
>   
> @@ -75,7 +77,8 @@ _require_scratch_write_atomic()
>   
>   	_scratch_unmount
>   
> -	if [ $awu_min_fs -eq 0 ] && [ $awu_max_fs -eq 0 ]; then
> +	if [ -z "$awu_min_fs" -o -z "$awu_max_fs" ] || \
> +		[ $awu_min_fs -eq 0 -a $awu_max_fs -eq 0 ];then
>   		_notrun "write atomic not supported by this filesystem"
>   	fi
>   }

LGTM

Reviewed-by: Pavel Reichl <preichl@redhat.com>

Re: [PATCH] common/atomicwrites: fix _require_scratch_write_atomic when awu_max_fs is empty

[Ojaswin Mujoo]

On Sat, Sep 06, 2025 at 09:17:22PM +0800, Zorro Lang wrote:
> If a kernel doesn't support atomic write, _get_atomic_write_unit_min
> and _get_atomic_write_unit_max will get nothing.
> 
>   local awu_min_bdev=$(_get_atomic_write_unit_min $SCRATCH_DEV)
>   local awu_max_bdev=$(_get_atomic_write_unit_max $SCRATCH_DEV)
> 
> If $awu_min_bdev and $awu_max_bdev are empty, then the logic likes
> "[ $awu_min_bdev -eq 0 ] && [ $awu_max_bdev -eq 0 ]" is meaningless,
> and it causes g/767 hang on system which doesn't support atomic
> write.
> 
> Signed-off-by: Zorro Lang <zlang@kernel.org>

I can confirm that this fixes the hang seen when xfs_io doesn't report
atomic write limits.

Feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Regards,
ojaswin

Re: [PATCH] common/atomicwrites: fix _require_scratch_write_atomic when awu_max_fs is empty

[Zorro Lang]

On Fri, Nov 14, 2025 at 03:54:00PM +0530, Ojaswin Mujoo wrote:
> On Sat, Sep 06, 2025 at 09:17:22PM +0800, Zorro Lang wrote:
> > If a kernel doesn't support atomic write, _get_atomic_write_unit_min
> > and _get_atomic_write_unit_max will get nothing.
> > 
> >   local awu_min_bdev=$(_get_atomic_write_unit_min $SCRATCH_DEV)
> >   local awu_max_bdev=$(_get_atomic_write_unit_max $SCRATCH_DEV)
> > 
> > If $awu_min_bdev and $awu_max_bdev are empty, then the logic likes
> > "[ $awu_min_bdev -eq 0 ] && [ $awu_max_bdev -eq 0 ]" is meaningless,
> > and it causes g/767 hang on system which doesn't support atomic
> > write.
> > 
> > Signed-off-by: Zorro Lang <zlang@kernel.org>
> 
> I can confirm that this fixes the hang seen when xfs_io doesn't report
> atomic write limits.
> 
> Feel free to add:
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Thanks Ojaswin, this patch has been merged, feel free to use the lastest
xfstests to avoid this issue :)

> 
> Regards,
> ojaswin
>