Hello Bart, On Mon, 2023-01-16 at 09:55 -0800, Bart Van Assche wrote: > On 1/16/23 06:59, Steffen Maier wrote: > > Hi all, > > > > since a few days/weeks, we sometimes see below alua and sleep > > related > > kernel BUG and WARNING (with panic_on_warn) in our CI. > > > > It reminds me of > > [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() > > https://lore.kernel.org/linux-scsi/166986602290.2101055.17397734326843853911.b4-ty@xxxxxxxxxx/ > > > > which I thought was the fix and went into 6.2-rc(1?) on 2022-12-14 > > with > > [GIT PULL] first round of SCSI updates for the 6.1+ merge window > > https://lore.kernel.org/linux-scsi/b2e824bbd1e40da64d2d01657f2f7a67b98919fb.camel@xxxxxxxxxxxxxxxxxxxxx/T/#u > > > > Due to limited history, I cannot tell exactly when problems started > > and > > whether it really correlates to above. > > > > Test workload are all kinds of coverage tests for zfcp recovery > > including scsi device removal and/or rescan. > > > > [ 4569.045992] BUG: sleeping function called from invalid context > > at > > drivers/scsi/device_handler/scsi_dh_alua.c:992 > > [ 4569.046003] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, > > pid: 0, > > name: swapper/8 > > [ 4569.046013] preempt_count: 101, expected: 0 > > [ 4569.046023] RCU nest depth: 0, expected: 0 > > [ 4569.046033] no locks held by swapper/8/0. > > [ 4569.046042] Preemption disabled at: > > Thanks, > > Bart. > > [ 4569.046046] [<000000017e27ce4e>] > > __slab_alloc.constprop.0+0x36/0xb8 > > [ 4569.046072] CPU: 8 PID: 0 Comm: swapper/8 Tainted: G W > > 6.2.0-20230114.rc3.git0.46e26dd43df0.300.fc37.s390x+debug #1 > > [ 4569.046084] Hardware name: IBM 2964 NC9 702 (z/VM 6.4.0) > > [ 4569.046094] Call Trace: > > [ 4569.046102] [<000000017ed21bcc>] dump_stack_lvl+0xac/0x100 > > [ 4569.046118] [<000000017df9192c>] __might_resched+0x284/0x2c8 > > [ 4569.046131] [<000003ff7fb9c874>] alua_rtpg_queue+0x3c/0x98 > > [scsi_dh_alua] > > [ 4569.046146] [<000003ff7fb9cfb2>] alua_check+0x122/0x250 > > [scsi_dh_alua] > > [ 4569.046167] [<000003ff7fb9d562>] alua_check_sense+0x172/0x228 > > [scsi_dh_alua] > > [ 4569.046179] [<000000017e96b3e2>] scsi_check_sense+0x8a/0x2e0 > > [ 4569.046191] [<000000017e96e4b6>] > > scsi_decide_disposition+0x286/0x298 > > [ 4569.046201] [<000000017e972bca>] scsi_complete+0x6a/0x108 > > [ 4569.046212] [<000000017e746906>] blk_complete_reqs+0x6e/0x88 > > [ 4569.046227] [<000000017ed3830e>] __do_softirq+0x13e/0x6b8 > > [ 4569.046238] [<000000017df57902>] __irq_exit_rcu+0x14a/0x170 > > [ 4569.046264] [<000000017df58472>] irq_exit_rcu+0x22/0x50 > > [ 4569.046275] [<000000017ed2242a>] do_ext_irq+0x10a/0x1d0 > > [ 4569.046286] [<000000017ed36156>] ext_int_handler+0xd6/0x110 > > [ 4569.046296] [<000000017ed362e6>] psw_idle_exit+0x0/0xa > > [ 4569.046307] ([<000000017defc5da>] arch_cpu_idle+0x52/0xe0) > > [ 4569.046318] [<000000017ed34744>] default_idle_call+0x84/0xd0 > > [ 4569.046329] [<000000017dfbe4cc>] do_idle+0xfc/0x1b8 > > [ 4569.046340] [<000000017dfbe80e>] cpu_startup_entry+0x36/0x40 > > [ 4569.046350] [<000000017df11964>] > > smp_start_secondary+0x14c/0x160 > > [ 4569.046371] [<000000017ed3658e>] restart_int_handler+0x6e/0x90 > > [ 4569.046381] no locks held by swapper/8/0. > Hi Steffen, > > Thanks for your report and also for having included this call trace. > Is > my understanding correct that alua_rtpg_queue+0x3c refers to the > might_sleep() near the start of alua_rtpg_queue()? If so, please help > with testing the following patch: > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c > b/drivers/scsi/device_handler/scsi_dh_alua.c > index 49cc18a87473..79afa7acdfbc 100644 > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > @@ -989,8 +989,6 @@ static bool alua_rtpg_queue(struct > alua_port_group > int start_queue = 0; > unsigned long flags; > > - might_sleep(); > - > if (WARN_ON_ONCE(!pg) || scsi_device_get(sdev)) > return false; > > > I'm proposing this change because the context from which a request is > queued should hold a reference on 'sdev' while a request is in > progress > so alua_check_sense() should not trigger the scsi_device_put() call > in > alua_rtpg_queue(). alua_rtpg_queue() must take an additional reference in order to make sure that the ref survives until the workqueue is started. A possible reference hold by the caller doesn't help because the caller might have dropped the ref before the workqueue runs. Please explain. Am I overlooking something? Regards Martin
