[bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests

[bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests

[Yi Zhang]

Hello
I found the scsi-debug module removing [1] takes more than 3mins
during blktests srp/ tests, and bisecting shows it was introduced from
[2],
Pls help check it, let me know if you need more info for it, thanks.

[1]
# time ./check srp/001
srp/001 (Create and remove LUNs)                             [passed]
    runtime    ...  3.194s
real 3m12.119s
user 0m0.859s
sys 0m2.227s

# ps aux | grep modprobe
root      250153  0.0  0.0  10600  2264 pts/0    D+   01:34   0:00
modprobe -r scsi_debug

# cat /proc/250153/stack
[<0>] blk_execute_rq+0x95/0xb0
[<0>] __scsi_execute+0xe2/0x250
[<0>] sd_sync_cache+0xac/0x190
[<0>] sd_shutdown+0x67/0xf0
[<0>] sd_remove+0x39/0x80
[<0>] __device_release_driver+0x234/0x240
[<0>] device_release_driver+0x23/0x30
[<0>] bus_remove_device+0xd8/0x140
[<0>] device_del+0x18b/0x3f0
[<0>] __scsi_remove_device+0x102/0x140
[<0>] scsi_forget_host+0x55/0x60
[<0>] scsi_remove_host+0x72/0x110
[<0>] sdebug_driver_remove+0x22/0xa0 [scsi_debug]
[<0>] __device_release_driver+0x181/0x240
[<0>] device_release_driver+0x23/0x30
[<0>] bus_remove_device+0xd8/0x140
[<0>] device_del+0x18b/0x3f0
[<0>] device_unregister+0x13/0x60
[<0>] sdebug_do_remove_host+0xd1/0xf0 [scsi_debug]
[<0>] scsi_debug_exit+0x58/0xe1e [scsi_debug]
[<0>] __do_sys_delete_module.constprop.0+0x170/0x260
[<0>] do_syscall_64+0x3a/0x80
[<0>] entry_SYSCALL_64_after_hwframe+0x44/0xae

# dmesg | tail -10
[  345.863755] ib_srpt:srpt_release_channel_work: ib_srpt 10.16.221.74-32
[  345.863855] ib_srpt:srpt_release_channel_work: ib_srpt 10.16.221.74-34
[  345.863953] ib_srpt:srpt_release_channel_work: ib_srpt 10.16.221.74-36
[  346.373371] sd 15:0:0:0: [sdb] Synchronizing SCSI cache
[  532.864536] sd 15:0:0:0: [sdb] Synchronize Cache(10) failed:
Result: hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK
------> seems most of the time were taken here
[  532.929626] eno1np0 speed is unknown, defaulting to 1000
[  532.938524] eno2np1 speed is unknown, defaulting to 1000
[  532.943957] eno4 speed is unknown, defaulting to 1000
[  532.998059] rdma_rxe: rxe-ah pool destroyed with unfree'd elem
[  533.011781] rdma_rxe: unloaded

[2]
commit 2aad3cd8537033cd34f70294a23f54623ffe9c1b (refs/bisect/bad)
Author: Douglas Gilbert <dgilbert@interlog.com>
Date:   Sat Jan 8 20:28:45 2022 -0500

    scsi: scsi_debug: Address races following module load

Re: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests

[Yi Zhang]

Ping

On Sat, Apr 2, 2022 at 3:06 PM Yi Zhang <yi.zhang@redhat.com> wrote:
>
> Hello
> I found the scsi-debug module removing [1] takes more than 3mins
> during blktests srp/ tests, and bisecting shows it was introduced from
> [2],
> Pls help check it, let me know if you need more info for it, thanks.
>
> [1]
> # time ./check srp/001
> srp/001 (Create and remove LUNs)                             [passed]
>     runtime    ...  3.194s
> real 3m12.119s
> user 0m0.859s
> sys 0m2.227s
>
> # ps aux | grep modprobe
> root      250153  0.0  0.0  10600  2264 pts/0    D+   01:34   0:00
> modprobe -r scsi_debug
>
> # cat /proc/250153/stack
> [<0>] blk_execute_rq+0x95/0xb0
> [<0>] __scsi_execute+0xe2/0x250
> [<0>] sd_sync_cache+0xac/0x190
> [<0>] sd_shutdown+0x67/0xf0
> [<0>] sd_remove+0x39/0x80
> [<0>] __device_release_driver+0x234/0x240
> [<0>] device_release_driver+0x23/0x30
> [<0>] bus_remove_device+0xd8/0x140
> [<0>] device_del+0x18b/0x3f0
> [<0>] __scsi_remove_device+0x102/0x140
> [<0>] scsi_forget_host+0x55/0x60
> [<0>] scsi_remove_host+0x72/0x110
> [<0>] sdebug_driver_remove+0x22/0xa0 [scsi_debug]
> [<0>] __device_release_driver+0x181/0x240
> [<0>] device_release_driver+0x23/0x30
> [<0>] bus_remove_device+0xd8/0x140
> [<0>] device_del+0x18b/0x3f0
> [<0>] device_unregister+0x13/0x60
> [<0>] sdebug_do_remove_host+0xd1/0xf0 [scsi_debug]
> [<0>] scsi_debug_exit+0x58/0xe1e [scsi_debug]
> [<0>] __do_sys_delete_module.constprop.0+0x170/0x260
> [<0>] do_syscall_64+0x3a/0x80
> [<0>] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> # dmesg | tail -10
> [  345.863755] ib_srpt:srpt_release_channel_work: ib_srpt 10.16.221.74-32
> [  345.863855] ib_srpt:srpt_release_channel_work: ib_srpt 10.16.221.74-34
> [  345.863953] ib_srpt:srpt_release_channel_work: ib_srpt 10.16.221.74-36
> [  346.373371] sd 15:0:0:0: [sdb] Synchronizing SCSI cache
> [  532.864536] sd 15:0:0:0: [sdb] Synchronize Cache(10) failed:
> Result: hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK
> ------> seems most of the time were taken here
> [  532.929626] eno1np0 speed is unknown, defaulting to 1000
> [  532.938524] eno2np1 speed is unknown, defaulting to 1000
> [  532.943957] eno4 speed is unknown, defaulting to 1000
> [  532.998059] rdma_rxe: rxe-ah pool destroyed with unfree'd elem
> [  533.011781] rdma_rxe: unloaded
>
> [2]
> commit 2aad3cd8537033cd34f70294a23f54623ffe9c1b (refs/bisect/bad)
> Author: Douglas Gilbert <dgilbert@interlog.com>
> Date:   Sat Jan 8 20:28:45 2022 -0500
>
>     scsi: scsi_debug: Address races following module load



--
Best Regards,
  Yi Zhang

scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Luis Chamberlain]

Moving this discussion to the lists as we need to really think
about how testing on fstests and blktests uses scsi_debug for
a high confidence in baseline without false positives on failures
due to the inability to the remove scsi_debug module.

This should also apply to other test debug modules like null_blk,
nvme target loop drivers, etc, it's all the same long term. But yeah
scsi surely make this... painful today. In any case hopefully folks
with other test debug drivesr are running tests to ensure you can
always rmmod these modules regardless of what is happening.

On Tue, Apr 12, 2022 at 06:03:40PM -0400, Douglas Gilbert wrote:
> On 2022-04-12 13:48, Luis Chamberlain wrote:
> > On Thu, Apr 07, 2022 at 10:09:54PM -0400, Douglas Gilbert wrote:
> > > Hi,
> > > Is it time to revert this patch?
> > 
> > Upstream kmod will indeed get patched soon witha  --patient-remove
> > option. So the issue is that. However, it doesn't mean driver's
> > can't / should strive to avoid these issues if they can. That is a
> > thing left to driver's to implement / resolve if they want.
> > 
> > In the meantime userspace should change to user the patient removal,
> > and if the upstream kmod doesn't have yet have it (note, the code is
> > not yet merged) then tools doing module removal should open code the
> > module removal. I modified fstests to do open coding of the patient
> > module removal in case kmod does not support it.  I have a similar patch
> > for blktests but that still requires regression testing on my part. I
> > hope to finish that soon though.
> > 
> > So the answer to your question: it depends on how well you want to deal
> > with these issues for users, or punt the problems to patient removal
> > usage.
> 
> Hi,
> There is a significant amount of work bringing down a driver like scsi_debug.
> Apart from potentially consuming most of the ram on a box, it also has the
> issue of SCSI commands that are "in flight" when rmmod is called.
> 
> So I think it is approaching impossible to make rmmod scsi_debug the equivalent
> of an atomic operation. There are just too many moving parts, potentially
> moving asynchronously to one another. This is an extremely good test for the
> SCSI/block system, roughly equivalent to losing a HBA that has a lot of disks
> behind it. Will the system stabilize and how long will that take?

I understand. But I really cannot buy "impossible". Impossible I think should
mean a design flaw somewhere.

At least for now I think we should narrow our objectives so that
this is *possible* within the context of fstests and blktests because
otherwise *we really should not be using scsi_debug* for high fidelity
in testing. One of the reasons is that we want to be able to run
fstests or blktests in a loop with confidence so that failures are
real. A failure due to the inability to not remove a debug module
makes gaining confidence in a baseline a bit difficult and you'd have
to implement hacks around it.

mcgrof@fulton ~/devel/blktests (git::master)$ git grep _have_scsi_debug tests | wc -l
10

mcgrof@fulton ~/devel/xfstests-dev (git::master)$ git grep _require_scsi_debug tests| wc -l
5

Not insane, but enough for us to care, but I think if we *narrow* our
scope to ensure scsi_debug *can* be removed *at least* with the patient
module remover we're good.

Do you think this is viable goal for scsi_debug?

> Setting up races between modprobe and rmmod on scsi_debug was certainly not
> top of mind for me.

Oh I get it. But the community has already embraced it for years on
fstests and blktests. So at this point I think we have no other option.

I think one thing we *can* do is *not* use scsi_debug for tests which
*really don't need scsi*.

> Storage systems such as SCSI are a lot better defined
> (and ordered) in the power-up scenario. Even with asynchronous scanning
> (discovery) of devices (even SSDs) it can take 10 plus seconds to bring up
> devices with a lot more handshaking between controller and the storage
> device. And even with SSDs, there is increased power draw during power-up
> (hard disks obviously need to accelerate the medium up to the rated speed).
> That leads to big storage arrays staggering when they apply power to
> different banks of SSDs/disks.

Sure..

> I wonder if anyone has tested building scsi_mod (the SCSI mid-level) as a
> module and tried rmmod on it while, say, a USB key is being read :-)

:)

  Luis

Re: scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Christoph Hellwig]

On Thu, Apr 21, 2022 at 10:53:30AM -0700, Luis Chamberlain wrote:
> Moving this discussion to the lists as we need to really think
> about how testing on fstests and blktests uses scsi_debug for
> a high confidence in baseline without false positives on failures
> due to the inability to the remove scsi_debug module.
> 
> This should also apply to other test debug modules like null_blk,
> nvme target loop drivers, etc, it's all the same long term. But yeah
> scsi surely make this... painful today. In any case hopefully folks
> with other test debug drivesr are running tests to ensure you can
> always rmmod these modules regardless of what is happening.

Maybe fix blktests to not rely on module removal  I have such a hard
time actually using blktests because it is suck a f^^Y% broken piece
of crap that assumes everything is modular.  Stop making that whole
assumption and work fine with built-in driver as a first step.  Then
start worrying about module removal.

Re: scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Theodore Ts'o]

On Thu, Apr 21, 2022 at 10:56:57PM -0700, Christoph Hellwig wrote:
> > This should also apply to other test debug modules like null_blk,
> > nvme target loop drivers, etc, it's all the same long term. But yeah
> > scsi surely make this... painful today. In any case hopefully folks
> > with other test debug drivesr are running tests to ensure you can
> > always rmmod these modules regardless of what is happening.
> 
> Maybe fix blktests to not rely on module removal  I have such a hard
> time actually using blktests because it is suck a f^^Y% broken piece
> of crap that assumes everything is modular.  Stop making that whole
> assumption and work fine with built-in driver as a first step.  Then
> start worrying about module removal.

I would love it if blktests didn't require modules, period.  That's
because it's super-convenient to be able to pluck a kernel out from
the build tree without having to install it first.  If all of the
necessary devices could be built-into the kernel, this would allow
this to work:

     make
     kvm-xfstests --blktests

which ends up running something like this:

/usr/bin/kvm -boot order=c -net none -machine type=pc,accel=kvm:tcg \
	     ... \
	     --kernel /build/ext4/arch/x86/boot/bzImage
	     --append "quiet loglevel=0 root=/dev/vda console=ttyS0,115200 nokaslr fstestopt=blktests,aex ..."

Unfortunately, because blktests requires modules, I can only do this
sort of thing using gce-xfstests and by passing in a kernel.deb file
so I can get the !@#@! modules installed.  (If I could use kexec with
gce-xfstets, I'd shave almost a minute of test appliance setup time,
and starting a GCE VM talks a minute or two extra over using
kvm-xfstests.)

I think we would need to make some changes to how scsi_debug and other
block device modules, first, though.  blktests is using modules
because that appears to be the only way to configure some of these
test/debug drivers and then have the debug device show up with the
specified characteristics.

						- Ted

Re: scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Luis Chamberlain]

On Thu, Apr 21, 2022 at 10:56:57PM -0700, Christoph Hellwig wrote:
> On Thu, Apr 21, 2022 at 10:53:30AM -0700, Luis Chamberlain wrote:
> > Moving this discussion to the lists as we need to really think
> > about how testing on fstests and blktests uses scsi_debug for
> > a high confidence in baseline without false positives on failures
> > due to the inability to the remove scsi_debug module.
> > 
> > This should also apply to other test debug modules like null_blk,
> > nvme target loop drivers, etc, it's all the same long term. But yeah
> > scsi surely make this... painful today. In any case hopefully folks
> > with other test debug drivesr are running tests to ensure you can
> > always rmmod these modules regardless of what is happening.
> 
> Maybe fix blktests to not rely on module removal  I have such a hard
> time actually using blktests because it is suck a f^^Y% broken piece
> of crap that assumes everything is modular.  Stop making that whole
> assumption and work fine with built-in driver as a first step.  Then
> start worrying about module removal.

It begs the question if the same wish should apply to fstests.

If we want to *not* rely on module removal then the right thing to do I
think would be to replace module removal on these debug modules
(scsi_debug) with an API as null_blk has which uses configfs to *add* /
*remove* devices.

  Luis

Re: scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Luis Chamberlain]

On Fri, Apr 22, 2022 at 08:34:12AM -0400, Theodore Ts'o wrote:
> I think we would need to make some changes to how scsi_debug and other
> block device modules, first, though.  blktests is using modules
> because that appears to be the only way to configure some of these
> test/debug drivers and then have the debug device show up with the
> specified characteristics.

Agreed.

  Luis

Re: scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Christoph Hellwig]

On Fri, Apr 22, 2022 at 08:34:12AM -0400, Theodore Ts'o wrote:
> I would love it if blktests didn't require modules, period.  That's
> because it's super-convenient to be able to pluck a kernel out from
> the build tree without having to install it first.  If all of the
> necessary devices could be built-into the kernel, this would allow
> this to work:

Yes, all my testing runs that way normally.  And running blktests
does not fit that workflow at all.

Re: scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Christoph Hellwig]

On Fri, Apr 22, 2022 at 08:06:33AM -0700, Luis Chamberlain wrote:
> It begs the question if the same wish should apply to fstests.

xfstests generally works fine except for a handful test that explicitly
want module removal but _notrun gracefully in that case.  Unlike
blktests which just explodes.

> If we want to *not* rely on module removal then the right thing to do I
> think would be to replace module removal on these debug modules
> (scsi_debug) with an API as null_blk has which uses configfs to *add* /
> *remove* devices.

scsi_debug has runtime writable module parameters that cover just
about everything.

Re: scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Douglas Gilbert]

On 2022-04-22 11:06, Luis Chamberlain wrote:
> On Thu, Apr 21, 2022 at 10:56:57PM -0700, Christoph Hellwig wrote:
>> On Thu, Apr 21, 2022 at 10:53:30AM -0700, Luis Chamberlain wrote:
>>> Moving this discussion to the lists as we need to really think
>>> about how testing on fstests and blktests uses scsi_debug for
>>> a high confidence in baseline without false positives on failures
>>> due to the inability to the remove scsi_debug module.
>>>
>>> This should also apply to other test debug modules like null_blk,
>>> nvme target loop drivers, etc, it's all the same long term. But yeah
>>> scsi surely make this... painful today. In any case hopefully folks
>>> with other test debug drivesr are running tests to ensure you can
>>> always rmmod these modules regardless of what is happening.
>>
>> Maybe fix blktests to not rely on module removal  I have such a hard
>> time actually using blktests because it is suck a f^^Y% broken piece
>> of crap that assumes everything is modular.  Stop making that whole
>> assumption and work fine with built-in driver as a first step.  Then
>> start worrying about module removal.
> 
> It begs the question if the same wish should apply to fstests.
> 
> If we want to *not* rely on module removal then the right thing to do I
> think would be to replace module removal on these debug modules
> (scsi_debug) with an API as null_blk has which uses configfs to *add* /
> *remove* devices.

The scsi_debug driver has been around for a while, I started maintaining it
around 1998. It was always assumed that it would be used as a module while
testing, then removed. You might wonder why the number of SCSI hosts simulated
by scsi_debug is called "add_hosts"? That is because it is also a sysfs
read-write parameter that can take a positive or negative integer to add or
remove that number of SCSI hosts at runtime, without removing the module.
See /sys/bus/pseudo/drivers/scsi_debug where about 2/3 of the parameters
are writable. Perhaps configfs capability could be added to scsi_debug,
patches welcome ...

If you want a cheap ram disk to back file system tests then null_blk is the
way to go.

The scsi_debug driver is a SCSI low level driver (LLD) controlling a
simulated HBA (on the "pseudo" bus) that has zero or more SCSI devices
(e.g. disks) attached to it. It is designed to back sd, st, ses Linux
devices (e.g. /dev/sdb). And it can simulate various types of storage
that are found in the real world, for example storage with associated
protection information and more recently with zoned storage. It can also
simulate errors and/or a _lot_ of devices (say 10,000) by sharing the
backing ram behind each device.

So the fact that the scsi_debug driver can support blktests could be seen
as a bit of an accident, that is not its primary purpose.

Doug Gilbert

Re: scsi_debug in fstests and blktests (Was: Re: Fwd: [bug report][bisected] modprob -r scsi-debug take more than 3mins during blktests srp/ tests)

[Chaitanya Kulkarni]

On 4/23/22 09:50, Christoph Hellwig wrote:
> On Fri, Apr 22, 2022 at 08:06:33AM -0700, Luis Chamberlain wrote:
>> It begs the question if the same wish should apply to fstests.
> 
> xfstests generally works fine except for a handful test that explicitly
> want module removal but _notrun gracefully in that case.  Unlike
> blktests which just explodes.
> 
>> If we want to *not* rely on module removal then the right thing to do I
>> think would be to replace module removal on these debug modules
>> (scsi_debug) with an API as null_blk has which uses configfs to *add* /
>> *remove* devices.
> 
> scsi_debug has runtime writable module parameters that cover just
> about everything.
> 

I've added to the my TODO list making nvme-blktests where we can avoid
the need for module loading and unloading, I'll also add a module
load-unload specific tests to that functionality will get tested.

-ck