[PATCH v2 blktests] block/002: remove debugfs check while blktests is running

[PATCH v2 blktests] block/002: remove debugfs check while blktests is running

[Yi Zhang]

See commit: 99d055b4fd4b ("block: remove per-disk debugfs files in blk_unregister_queue")

Signed-off-by: Yi Zhang <yi.zhang@redhat.com>
---
 tests/block/002 | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tests/block/002 b/tests/block/002
index 9b183e7..15b47a6 100755
--- a/tests/block/002
+++ b/tests/block/002
@@ -25,9 +25,6 @@ test() {
 	blktrace -D "$TMP_DIR" "/dev/${SCSI_DEBUG_DEVICES[0]}" >"$FULL" 2>&1 &
 	sleep 0.5
 	echo 1 > "/sys/block/${SCSI_DEBUG_DEVICES[0]}/device/delete"
-	if [[ ! -d /sys/kernel/debug/block/${SCSI_DEBUG_DEVICES[0]} ]]; then
-		echo "debugfs directory deleted with blktrace active"
-	fi
 	{ kill $!; wait; } >/dev/null 2>/dev/null
 	if [[ -d /sys/kernel/debug/block/${SCSI_DEBUG_DEVICES[0]} ]]; then
 		echo "debugfs directory leaked"
-- 
2.34.1

Re: [PATCH v2 blktests] block/002: remove debugfs check while blktests is running

[Shinichiro Kawasaki]

Hi Yi, thanks for this patch. I've been seeing the block/002 failure for a
while. Happy to see it resolved.

On Jul 30, 2022 / 15:58, Yi Zhang wrote:
> See commit: 99d055b4fd4b ("block: remove per-disk debugfs files in blk_unregister_queue")
> 
> Signed-off-by: Yi Zhang <yi.zhang@redhat.com>
> ---
>  tests/block/002 | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/tests/block/002 b/tests/block/002
> index 9b183e7..15b47a6 100755
> --- a/tests/block/002
> +++ b/tests/block/002
> @@ -25,9 +25,6 @@ test() {
>  	blktrace -D "$TMP_DIR" "/dev/${SCSI_DEBUG_DEVICES[0]}" >"$FULL" 2>&1 &
>  	sleep 0.5
>  	echo 1 > "/sys/block/${SCSI_DEBUG_DEVICES[0]}/device/delete"
> -	if [[ ! -d /sys/kernel/debug/block/${SCSI_DEBUG_DEVICES[0]} ]]; then
> -		echo "debugfs directory deleted with blktrace active"
> -	fi

Regarding the commit subject, I think the word "blktests" is to be replaced with
"blktrace". As the removed echo command above shows, this patch removes the
check while blktrace is running.

Also, I suggest to add one more sentence in the commit message for recording
purpose:

  This fix avoids the test case failure observed since the kernel commit
  0a9a25ca7843 ("block: let blkcg_gq grab request queue's refcnt").

If you don't mind, I will do the edits above when I apply this v2 patch.

-- 
Shin'ichiro Kawasaki

Re: [PATCH v2 blktests] block/002: remove debugfs check while blktests is running

[Yi Zhang]

On Mon, Aug 1, 2022 at 9:30 AM Shinichiro Kawasaki
<shinichiro.kawasaki@wdc.com> wrote:
>
> Hi Yi, thanks for this patch. I've been seeing the block/002 failure for a
> while. Happy to see it resolved.
>
> On Jul 30, 2022 / 15:58, Yi Zhang wrote:
> > See commit: 99d055b4fd4b ("block: remove per-disk debugfs files in blk_unregister_queue")
> >
> > Signed-off-by: Yi Zhang <yi.zhang@redhat.com>
> > ---
> >  tests/block/002 | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/tests/block/002 b/tests/block/002
> > index 9b183e7..15b47a6 100755
> > --- a/tests/block/002
> > +++ b/tests/block/002
> > @@ -25,9 +25,6 @@ test() {
> >       blktrace -D "$TMP_DIR" "/dev/${SCSI_DEBUG_DEVICES[0]}" >"$FULL" 2>&1 &
> >       sleep 0.5
> >       echo 1 > "/sys/block/${SCSI_DEBUG_DEVICES[0]}/device/delete"
> > -     if [[ ! -d /sys/kernel/debug/block/${SCSI_DEBUG_DEVICES[0]} ]]; then
> > -             echo "debugfs directory deleted with blktrace active"
> > -     fi
>
> Regarding the commit subject, I think the word "blktests" is to be replaced with
> "blktrace". As the removed echo command above shows, this patch removes the
> check while blktrace is running.

Agree, thanks.

>
> Also, I suggest to add one more sentence in the commit message for recording
> purpose:
>
>   This fix avoids the test case failure observed since the kernel commit
>   0a9a25ca7843 ("block: let blkcg_gq grab request queue's refcnt").
>
> If you don't mind, I will do the edits above when I apply this v2 patch.

Sure, feel free to help update the commit message when you apply it, thanks. :)

>
> --
> Shin'ichiro Kawasaki
>


-- 
Best Regards,
  Yi Zhang

Re: [PATCH v2 blktests] block/002: remove debugfs check while blktests is running

[Shinichiro Kawasaki]

On Aug 01, 2022 / 09:35, Yi Zhang wrote:
> On Mon, Aug 1, 2022 at 9:30 AM Shinichiro Kawasaki
> <shinichiro.kawasaki@wdc.com> wrote:
> >
> > Hi Yi, thanks for this patch. I've been seeing the block/002 failure for a
> > while. Happy to see it resolved.
> >
> > On Jul 30, 2022 / 15:58, Yi Zhang wrote:
> > > See commit: 99d055b4fd4b ("block: remove per-disk debugfs files in blk_unregister_queue")
> > >
> > > Signed-off-by: Yi Zhang <yi.zhang@redhat.com>
> > > ---
> > >  tests/block/002 | 3 ---
> > >  1 file changed, 3 deletions(-)
> > >
> > > diff --git a/tests/block/002 b/tests/block/002
> > > index 9b183e7..15b47a6 100755
> > > --- a/tests/block/002
> > > +++ b/tests/block/002
> > > @@ -25,9 +25,6 @@ test() {
> > >       blktrace -D "$TMP_DIR" "/dev/${SCSI_DEBUG_DEVICES[0]}" >"$FULL" 2>&1 &
> > >       sleep 0.5
> > >       echo 1 > "/sys/block/${SCSI_DEBUG_DEVICES[0]}/device/delete"
> > > -     if [[ ! -d /sys/kernel/debug/block/${SCSI_DEBUG_DEVICES[0]} ]]; then
> > > -             echo "debugfs directory deleted with blktrace active"
> > > -     fi
> >
> > Regarding the commit subject, I think the word "blktests" is to be replaced with
> > "blktrace". As the removed echo command above shows, this patch removes the
> > check while blktrace is running.
> 
> Agree, thanks.
> 
> >
> > Also, I suggest to add one more sentence in the commit message for recording
> > purpose:
> >
> >   This fix avoids the test case failure observed since the kernel commit
> >   0a9a25ca7843 ("block: let blkcg_gq grab request queue's refcnt").
> >
> > If you don't mind, I will do the edits above when I apply this v2 patch.
> 
> Sure, feel free to help update the commit message when you apply it, thanks. :)

Thanks! Applied with some edits in the commit message.

-- 
Shin'ichiro Kawasaki

Re: [PATCH v2 blktests] block/002: remove debugfs check while blktests is running

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>