linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] common/rc: Modify _require_batched_discard to improve test coverage
@ 2022-05-16  8:45 Ojaswin Mujoo
  2022-06-13  8:35 ` Ojaswin Mujoo
  0 siblings, 1 reply; 4+ messages in thread
From: Ojaswin Mujoo @ 2022-05-16  8:45 UTC (permalink / raw)
  To: fstests; +Cc: zlang, riteshh, linux-ext4, linux-kernel

A recent ext4 patch discussed [1] that some devices (eg LVMs) can
have a discard granularity as big as 42MB which makes it larger
than the group size of ext4 FS with 1k BS.  This causes the FITRIM
IOCTL to fail.

This case was not correctly handled by this test since
"_require_batched_discard" incorrectly interpreted the FITRIM
failure as SCRATCH_DEV not supporting the IOCTL. This caused the test
to report "not run" instead of "failed" in case of large discard granularity.

Fix "_require_batched_discard" to use a more accurate method
to determine if discard is supported.

[1] commit 173b6e383d2
    ext4: avoid trim error on fs with small groups

Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>
---

Changes since v2 [1] 

*  Eliminated redundant $ret variable

[1]
https://lore.kernel.org/all/20220516063951.87838-1-ojaswin@linux.ibm.com/

 common/rc | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index e2d3d72a..94344248 100644
--- a/common/rc
+++ b/common/rc
@@ -3858,7 +3858,12 @@ _require_batched_discard()
 		exit 1
 	fi
 	_require_fstrim
-	$FSTRIM_PROG $1 > /dev/null 2>&1 || _notrun "FITRIM not supported on $1"
+
+	grep -q "not supported" <($FSTRIM_PROG $1 2>&1)
+	if [ "$?" = "0" ]
+	then
+		_notrun "FITRIM not supported on $1"
+	fi
 }
 
 _require_dumpe2fs()
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] common/rc: Modify _require_batched_discard to improve test coverage
  2022-05-16  8:45 [PATCH v3] common/rc: Modify _require_batched_discard to improve test coverage Ojaswin Mujoo
@ 2022-06-13  8:35 ` Ojaswin Mujoo
  2022-06-15  9:56   ` Zorro Lang
  0 siblings, 1 reply; 4+ messages in thread
From: Ojaswin Mujoo @ 2022-06-13  8:35 UTC (permalink / raw)
  To: fstests; +Cc: zlang, riteshh, linux-ext4, linux-kernel

Greetings,

Please let me know if there are any reviews/suggestions on this
patch.

Thank you!

On Mon, May 16, 2022 at 02:15:05PM +0530, Ojaswin Mujoo wrote:
> A recent ext4 patch discussed [1] that some devices (eg LVMs) can
> have a discard granularity as big as 42MB which makes it larger
> than the group size of ext4 FS with 1k BS.  This causes the FITRIM
> IOCTL to fail.
> 
> This case was not correctly handled by this test since
> "_require_batched_discard" incorrectly interpreted the FITRIM
> failure as SCRATCH_DEV not supporting the IOCTL. This caused the test
> to report "not run" instead of "failed" in case of large discard granularity.
> 
> Fix "_require_batched_discard" to use a more accurate method
> to determine if discard is supported.
> 
> [1] commit 173b6e383d2
>     ext4: avoid trim error on fs with small groups
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
> 
> Changes since v2 [1] 
> 
> *  Eliminated redundant $ret variable
> 
> [1]
> https://lore.kernel.org/all/20220516063951.87838-1-ojaswin@linux.ibm.com/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] common/rc: Modify _require_batched_discard to improve test coverage
  2022-06-13  8:35 ` Ojaswin Mujoo
@ 2022-06-15  9:56   ` Zorro Lang
  2022-06-20  5:05     ` Ojaswin Mujoo
  0 siblings, 1 reply; 4+ messages in thread
From: Zorro Lang @ 2022-06-15  9:56 UTC (permalink / raw)
  To: Ojaswin Mujoo; +Cc: fstests, zlang, riteshh, linux-ext4, linux-kernel

On Mon, Jun 13, 2022 at 02:05:34PM +0530, Ojaswin Mujoo wrote:
> Greetings,
> 
> Please let me know if there are any reviews/suggestions on this
> patch.

This patch has been merged in fstests v2022.05.22, as below

  ee264b3f common/rc: Modify _require_batched_discard to improve test coverage

Is there anything wrong with that?

Thanks,
Zorro

> 
> Thank you!
> 
> On Mon, May 16, 2022 at 02:15:05PM +0530, Ojaswin Mujoo wrote:
> > A recent ext4 patch discussed [1] that some devices (eg LVMs) can
> > have a discard granularity as big as 42MB which makes it larger
> > than the group size of ext4 FS with 1k BS.  This causes the FITRIM
> > IOCTL to fail.
> > 
> > This case was not correctly handled by this test since
> > "_require_batched_discard" incorrectly interpreted the FITRIM
> > failure as SCRATCH_DEV not supporting the IOCTL. This caused the test
> > to report "not run" instead of "failed" in case of large discard granularity.
> > 
> > Fix "_require_batched_discard" to use a more accurate method
> > to determine if discard is supported.
> > 
> > [1] commit 173b6e383d2
> >     ext4: avoid trim error on fs with small groups
> > 
> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > ---
> > 
> > Changes since v2 [1] 
> > 
> > *  Eliminated redundant $ret variable
> > 
> > [1]
> > https://lore.kernel.org/all/20220516063951.87838-1-ojaswin@linux.ibm.com/
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] common/rc: Modify _require_batched_discard to improve test coverage
  2022-06-15  9:56   ` Zorro Lang
@ 2022-06-20  5:05     ` Ojaswin Mujoo
  0 siblings, 0 replies; 4+ messages in thread
From: Ojaswin Mujoo @ 2022-06-20  5:05 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests, zlang, riteshh, linux-ext4, linux-kernel

On Wed, Jun 15, 2022 at 05:56:07PM +0800, Zorro Lang wrote:
> On Mon, Jun 13, 2022 at 02:05:34PM +0530, Ojaswin Mujoo wrote:
> > Greetings,
> > 
> > Please let me know if there are any reviews/suggestions on this
> > patch.
> 
> This patch has been merged in fstests v2022.05.22, as below
> 
>   ee264b3f common/rc: Modify _require_batched_discard to improve test coverage
> 
> Is there anything wrong with that?
> 
> Thanks,
> Zorro
> 

Ahh I wasn't aware of that, thanks for picking this up :)

Regards,
Ojaswin

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-06-20  5:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-16  8:45 [PATCH v3] common/rc: Modify _require_batched_discard to improve test coverage Ojaswin Mujoo
2022-06-13  8:35 ` Ojaswin Mujoo
2022-06-15  9:56   ` Zorro Lang
2022-06-20  5:05     ` Ojaswin Mujoo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).