| Message ID | 20180321153854.GB24312@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
[ Adding PeterZ to participants due to query about lockdep_assert() ] On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > assert_spin_locked(&cmd->t_state_lock); > - WARN_ON_ONCE(!irqs_disabled()); > + WARN_ON_ONCE_NONRT(!irqs_disabled()); Ugh. Can't we just replace both of those with a lockdep annotation? Does "lockdep_assert_held()" already verify the irq contextr, or do we need lockdep_assert_irqs_disabled() too? Honestly, the old-fashioned way of doing verification of state by hand is understandable, but it's legacy and kind of pointless when we have much better tools these days. I'm perfectly willing to leave old assertions in place, but if they need fixing anyway, I'd damn well want to fix them *right* instead of starting to just add more piles of hacks on top of the old model. Because when the details of the locking rules depend on RT vs non-RT, I want the checks to make sense. And presumably lockdep is the thing that really knows what the status of a lock is, no? Linus -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 21, 2018 at 12:38:54PM -0300, Arnaldo Carvalho de Melo wrote: > assert_spin_locked(&cmd->t_state_lock); > - WARN_ON_ONCE(!irqs_disabled()); > + WARN_ON_ONCE_NONRT(!irqs_disabled()); I can't find where WARN_ON_ONCE_NONRT is defined. That being said I think we can just kill these asserts. If we have irqs disabled spin_unlock_irq a few lines below should already warn. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 21 Mar 2018 11:50:01 -0700 Christoph Hellwig <hch@infradead.org> wrote: > On Wed, Mar 21, 2018 at 12:38:54PM -0300, Arnaldo Carvalho de Melo wrote: > > assert_spin_locked(&cmd->t_state_lock); > > - WARN_ON_ONCE(!irqs_disabled()); > > + WARN_ON_ONCE_NONRT(!irqs_disabled()); > > I can't find where WARN_ON_ONCE_NONRT is defined. It's only in the RT patch set. But that may be changing soon. > > That being said I think we can just kill these asserts. If we have irqs > disabled spin_unlock_irq a few lines below should already warn. I agree with Linus. This should just be some kind of lockdep_assert_held_irqs_disabeld() or something like that. -- Steve -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 21 Mar 2018, Linus Torvalds wrote: > [ Adding PeterZ to participants due to query about lockdep_assert() ] > > On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > assert_spin_locked(&cmd->t_state_lock); > > - WARN_ON_ONCE(!irqs_disabled()); > > + WARN_ON_ONCE_NONRT(!irqs_disabled()); > > Ugh. > > Can't we just replace both of those with a lockdep annotation? > > Does "lockdep_assert_held()" already verify the irq contextr, or do we > need lockdep_assert_irqs_disabled() too? > > Honestly, the old-fashioned way of doing verification of state by hand > is understandable, but it's legacy and kind of pointless when we have > much better tools these days. > > I'm perfectly willing to leave old assertions in place, but if they > need fixing anyway, I'd damn well want to fix them *right* instead of > starting to just add more piles of hacks on top of the old model. > > Because when the details of the locking rules depend on RT vs non-RT, > I want the checks to make sense. And presumably lockdep is the thing > that really knows what the status of a lock is, no? We are working on replacing the _NONRT _RT variants with proper lockdep mechnisms which are aware of the RT vs. non-RT magic under the hood. Just not there yet. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Wed, Mar 21, 2018 at 11:43:58AM -0700, Linus Torvalds escreveu: > [ Adding PeterZ to participants due to query about lockdep_assert() ] > > On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > assert_spin_locked(&cmd->t_state_lock); > > - WARN_ON_ONCE(!irqs_disabled()); > > + WARN_ON_ONCE_NONRT(!irqs_disabled()); > > Ugh. > > Can't we just replace both of those with a lockdep annotation? Huh, even better, when that feature gets finished (tglx said it was being developed, not there yet tho) it'll allow further reduction of the PREEMPT_RT_FULL patchkit. - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Thu, Mar 22, 2018 at 06:37:45AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Wed, Mar 21, 2018 at 11:43:58AM -0700, Linus Torvalds escreveu: > > [ Adding PeterZ to participants due to query about lockdep_assert() ] > > > > On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo > > <acme@kernel.org> wrote: > > > > > > assert_spin_locked(&cmd->t_state_lock); > > > - WARN_ON_ONCE(!irqs_disabled()); > > > + WARN_ON_ONCE_NONRT(!irqs_disabled()); > > > > Ugh. > > > > Can't we just replace both of those with a lockdep annotation? > > Huh, even better, when that feature gets finished (tglx said it was > being developed, not there yet tho) it'll allow further reduction of the > PREEMPT_RT_FULL patchkit. We'd get rid of these: [acme@jouet patches-4.11.12-rt15]$ grep "^+[[:space:]]\+.*NONRT" *.patch dm-make-rt-aware.patch:+ BUG_ON_NONRT(!irqs_disabled()); fs-block-rt-support.patch:+ WARN_ON_NONRT(!irqs_disabled()); i915-bogus-warning-from-i915-when-running-on-PREEMPT.patch:+ WARN_ON_NONRT(!in_interrupt()); iommu-amd--Use-WARN_ON_NORT.patch:+ WARN_ON_NONRT(!irqs_disabled()); iommu-amd--Use-WARN_ON_NORT.patch:+ WARN_ON_NONRT(!irqs_disabled()); irqwork-push_most_work_into_softirq_context.patch:+ BUG_ON_NONRT(!irqs_disabled()); net-wireless-warn-nort.patch:+ WARN_ON_ONCE_NONRT(softirq_count() == 0); posix-timers-thread-posix-cpu-timers-on-rt.patch:+ WARN_ON_ONCE_NONRT(!irqs_disabled()); posix-timers-thread-posix-cpu-timers-on-rt.patch:+ WARN_ON_ONCE_NONRT(!irqs_disabled()); posix-timers-thread-posix-cpu-timers-on-rt.patch:+ WARN_ON_ONCE_NONRT(!irqs_disabled()); workqueue-use-locallock.patch:+ WARN_ON_ONCE_NONRT(!irqs_disabled()); [acme@jouet patches-4.11.12-rt15]$ - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 22 Mar 2018, Arnaldo Carvalho de Melo wrote: > Em Wed, Mar 21, 2018 at 11:43:58AM -0700, Linus Torvalds escreveu: > > [ Adding PeterZ to participants due to query about lockdep_assert() ] > > > > On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo > > <acme@kernel.org> wrote: > > > > > > assert_spin_locked(&cmd->t_state_lock); > > > - WARN_ON_ONCE(!irqs_disabled()); > > > + WARN_ON_ONCE_NONRT(!irqs_disabled()); > > > > Ugh. > > > > Can't we just replace both of those with a lockdep annotation? > > Huh, even better, when that feature gets finished (tglx said it was > being developed, not there yet tho) it'll allow further reduction of the > PREEMPT_RT_FULL patchkit. That's the evil plan :) -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arnaldo,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc6 next-20180322]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Arnaldo-Carvalho-de-Melo/target-Use-WARNON_NON_RT-irqs_disabled/20180322-174549
config: i386-randconfig-x015-201811 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers//target/target_core_transport.c: In function '__transport_check_aborted_status':
>> drivers//target/target_core_transport.c:3207:2: error: implicit declaration of function 'WARN_ON_ONCE_NONRT'; did you mean 'WARN_ON_ONCE'? [-Werror=implicit-function-declaration]
WARN_ON_ONCE_NONRT(!irqs_disabled());
^~~~~~~~~~~~~~~~~~
WARN_ON_ONCE
cc1: some warnings being treated as errors
vim +3207 drivers//target/target_core_transport.c
3199
3200 static int __transport_check_aborted_status(struct se_cmd *cmd, int send_status)
3201 __releases(&cmd->t_state_lock)
3202 __acquires(&cmd->t_state_lock)
3203 {
3204 int ret;
3205
3206 assert_spin_locked(&cmd->t_state_lock);
> 3207 WARN_ON_ONCE_NONRT(!irqs_disabled());
3208
3209 if (!(cmd->transport_state & CMD_T_ABORTED))
3210 return 0;
3211 /*
3212 * If cmd has been aborted but either no status is to be sent or it has
3213 * already been sent, just return
3214 */
3215 if (!send_status || !(cmd->se_cmd_flags & SCF_SEND_DELAYED_TAS)) {
3216 if (send_status)
3217 cmd->se_cmd_flags |= SCF_SEND_DELAYED_TAS;
3218 return 1;
3219 }
3220
3221 pr_debug("Sending delayed SAM_STAT_TASK_ABORTED status for CDB:"
3222 " 0x%02x ITT: 0x%08llx\n", cmd->t_task_cdb[0], cmd->tag);
3223
3224 cmd->se_cmd_flags &= ~SCF_SEND_DELAYED_TAS;
3225 cmd->scsi_status = SAM_STAT_TASK_ABORTED;
3226 trace_target_cmd_complete(cmd);
3227
3228 spin_unlock_irq(&cmd->t_state_lock);
3229 ret = cmd->se_tfo->queue_status(cmd);
3230 if (ret)
3231 transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
3232 spin_lock_irq(&cmd->t_state_lock);
3233
3234 return 1;
3235 }
3236
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Arnaldo,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc6 next-20180322]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Arnaldo-Carvalho-de-Melo/target-Use-WARNON_NON_RT-irqs_disabled/20180322-174549
config: i386-randconfig-s1-03221113 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/target/target_core_transport.c: In function '__transport_check_aborted_status':
>> drivers/target/target_core_transport.c:3207:2: error: implicit declaration of function 'WARN_ON_ONCE_NONRT' [-Werror=implicit-function-declaration]
WARN_ON_ONCE_NONRT(!irqs_disabled());
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/WARN_ON_ONCE_NONRT +3207 drivers/target/target_core_transport.c
3199
3200 static int __transport_check_aborted_status(struct se_cmd *cmd, int send_status)
3201 __releases(&cmd->t_state_lock)
3202 __acquires(&cmd->t_state_lock)
3203 {
3204 int ret;
3205
3206 assert_spin_locked(&cmd->t_state_lock);
> 3207 WARN_ON_ONCE_NONRT(!irqs_disabled());
3208
3209 if (!(cmd->transport_state & CMD_T_ABORTED))
3210 return 0;
3211 /*
3212 * If cmd has been aborted but either no status is to be sent or it has
3213 * already been sent, just return
3214 */
3215 if (!send_status || !(cmd->se_cmd_flags & SCF_SEND_DELAYED_TAS)) {
3216 if (send_status)
3217 cmd->se_cmd_flags |= SCF_SEND_DELAYED_TAS;
3218 return 1;
3219 }
3220
3221 pr_debug("Sending delayed SAM_STAT_TASK_ABORTED status for CDB:"
3222 " 0x%02x ITT: 0x%08llx\n", cmd->t_task_cdb[0], cmd->tag);
3223
3224 cmd->se_cmd_flags &= ~SCF_SEND_DELAYED_TAS;
3225 cmd->scsi_status = SAM_STAT_TASK_ABORTED;
3226 trace_target_cmd_complete(cmd);
3227
3228 spin_unlock_irq(&cmd->t_state_lock);
3229 ret = cmd->se_tfo->queue_status(cmd);
3230 if (ret)
3231 transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
3232 spin_lock_irq(&cmd->t_state_lock);
3233
3234 return 1;
3235 }
3236
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 2018-03-22 06:37:45 [-0300], Arnaldo Carvalho de Melo wrote: > Em Wed, Mar 21, 2018 at 11:43:58AM -0700, Linus Torvalds escreveu: > > [ Adding PeterZ to participants due to query about lockdep_assert() ] > > > > On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo > > <acme@kernel.org> wrote: > > > > > > assert_spin_locked(&cmd->t_state_lock); > > > - WARN_ON_ONCE(!irqs_disabled()); > > > + WARN_ON_ONCE_NONRT(!irqs_disabled()); > > > > Ugh. > > > > Can't we just replace both of those with a lockdep annotation? > > Huh, even better, when that feature gets finished (tglx said it was > being developed, not there yet tho) it'll allow further reduction of the > PREEMPT_RT_FULL patchkit. I am going take this into -RT tree for now until we have different solution. I will try to be kind and do the same change in __transport_wait_for_tasks(). Arnaldo, please do "[PATCH RT]" while sending patches. Then the bots don't complain if it applies but does not compile on !RT kernel (or so I've been told). Technically speaking the code wants to ensure that the lock is held and the interrupts are disabled because the lock is always taken with disabled interrupts. This kind of check could be done with lockdep_assert_held(&cmd->t_state_lock); but would require lockdep to be switched on. Nicholas, would you mind such a change? > - Arnaldo Sebastian -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2018-03-23 at 16:55 +0100, Sebastian Andrzej Siewior wrote: > I am going take this into -RT tree for now until we have different > solution. Have you considered to delete the WARN_ON_ONCE(!irqs_disabled()) statement? I think that check duplicates functionality that already exists in lockdep since lockdep is already able to detect spinlock use inconsistencies. Bart.
On 2018-03-23 16:25:25 [+0000], Bart Van Assche wrote: > On Fri, 2018-03-23 at 16:55 +0100, Sebastian Andrzej Siewior wrote: > > I am going take this into -RT tree for now until we have different > > solution. > > Have you considered to delete the WARN_ON_ONCE(!irqs_disabled()) statement? > I think that check duplicates functionality that already exists in lockdep > since lockdep is already able to detect spinlock use inconsistencies. correct. That is why I suggested to use lockdep_assert_held() instead of this IRQ-check + the spin_lock_assert(). The only downside is that this code (as of now) works with lockdep disabled. On the other hand, lockdep_assert_held() gives you a splat instead of a BUG() statement like spin_lock_assert() does so I clearly promote lockdep here :) > Bart. Sebastian -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 23, 2018 at 9:25 AM, Bart Van Assche <Bart.VanAssche@wdc.com> wrote: > > Have you considered to delete the WARN_ON_ONCE(!irqs_disabled()) statement? > I think that check duplicates functionality that already exists in lockdep > since lockdep is already able to detect spinlock use inconsistencies. Please just delete both lines. There is exactly two callers of that static function, and both of them do spin_lock_irq(&cmd->t_state_lock); right above the call. It's not like this is some function that is exported to random users, and we should check that the calling convention is right. So honestly, even lockdep annotations look like you don't need them. This looks like "it may have been useful during coding to document things, but it's not useful long-term". Sure, the annotation is not wrong, but even if you go "verification is good", you should ask yourself whether there are maybe better places that would catch more relevant problems, than putting verification into some static function with two trivially correct callers wrt this verification? Linus -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Fri, Mar 23, 2018 at 04:55:13PM +0100, Sebastian Andrzej Siewior escreveu: > Arnaldo, please do "[PATCH RT]" while sending patches. Then the bots > don't complain if it applies but does not compile on !RT kernel (or so > I've been told). Ok, I'll try to remember that for future patches. - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 4558f2e1fe1b..318453e7adfd 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3204,7 +3204,7 @@ static int __transport_check_aborted_status(struct se_cmd *cmd, int send_status) int ret; assert_spin_locked(&cmd->t_state_lock); - WARN_ON_ONCE(!irqs_disabled()); + WARN_ON_ONCE_NONRT(!irqs_disabled()); if (!(cmd->transport_state & CMD_T_ABORTED)) return 0;
Hi, We got a report where this WARN_ON got triggered: [ 7552.799997] ------------[ cut here ]------------ [ 7552.800016] WARNING: CPU: 7 PID: 1090 at drivers/target/target_core_transport.c:3009 __transport_check_aborted_status+0x153/0x190 [target_core_mod] [ 7552.800037] Modules linked in: target_core_user uio target_core_pscsi target_core_file target_core_iblock ib_srpt ib_srp scsi_transport_srp scsi_tgt xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 tun ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter bridge stp llc ib_isert iscsi_target_mod target_core_mod ib_ucm rpcrdma mlx5_ib sunrpc rdma_ucm ib_uverbs ib_iser rdma_cm iw_cm libiscsi ib_umad ib_ipoib scsi_transport_iscsi ib_cm sb_edac intel_powerclamp coretemp intel_rapl iosf_mbi crc32_pclmul ghash_clmulni_intel aesni_intel lrw gf128mul glue_helper ablk_helper cryptd iTCO_wdt iTCO_vendor_support hfi1 ipmi_ssif sg rdmavt ib_core hpilo hpwdt pcspkr ipmi_si [ 7552.800055] ipmi_devintf ipmi_msghandler wmi acpi_power_meter ioatdma dca shpchp pcc_cpufreq lpc_ich ip_tables xfs libcrc32c mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm sd_mod crc_t10dif crct10dif_generic crct10dif_pclmul crct10dif_common crc32c_intel serio_raw ata_generic pata_acpi mlx5_core ata_piix tg3 drm devlink libata i2c_core ptp hpsa scsi_transport_sas pps_core dm_mirror dm_region_hash dm_log dm_mod [last unloaded: ib_srpt] [ 7552.800058] CPU: 7 PID: 1090 Comm: kworker/7:1H Not tainted 3.10.0-768.rt56.699.el7.x86_64 #1 [ 7552.800058] Hardware name: HP ProLiant DL380p Gen8, BIOS P70 11/14/2013 [ 7552.800066] Workqueue: ib-comp-wq ib_cq_poll_work [ib_core] [ 7552.800067] Call Trace: [ 7552.800075] [<ffffffffb76cd055>] dump_stack+0x19/0x1b [ 7552.800078] [<ffffffffb70807bb>] __warn+0xfb/0x120 [ 7552.800080] [<ffffffffb70808fd>] warn_slowpath_null+0x1d/0x20 [ 7552.800085] [<ffffffffc0ab3983>] __transport_check_aborted_status+0x153/0x190 [target_core_mod] [ 7552.800091] [<ffffffffc0ab5c04>] target_execute_cmd+0x34/0x2e0 [target_core_mod] [ 7552.800096] [<ffffffffc0ab5fc2>] transport_generic_new_cmd+0x112/0x240 [target_core_mod] [ 7552.800100] [<ffffffffc0ab6132>] transport_handle_cdb_direct+0x42/0x90 [target_core_mod] [ 7552.800105] [<ffffffffc0ab62cd>] target_submit_cmd_map_sgls+0x14d/0x210 [target_core_mod] [ 7552.800107] [<ffffffffc09c15b4>] srpt_handle_new_iu+0x254/0x660 [ib_srpt] [ 7552.800109] [<ffffffffc09c1bc8>] srpt_recv_done+0x38/0x60 [ib_srpt] [ 7552.800113] [<ffffffffc07a5fb5>] __ib_process_cq+0x65/0xe0 [ib_core] [ 7552.800118] [<ffffffffc07a60a0>] ib_cq_poll_work+0x20/0x60 [ib_core] [ 7552.800120] [<ffffffffb70a4336>] process_one_work+0x176/0x4a0 [ 7552.800121] [<ffffffffb70a50ec>] worker_thread+0x16c/0x3f0 [ 7552.800123] [<ffffffffb70a4f80>] ? manage_workers.isra.36+0x2b0/0x2b0 [ 7552.800125] [<ffffffffb70ac62f>] kthread+0xcf/0xe0 [ 7552.800139] [<ffffffffb70ac560>] ? kthread_worker_fn+0x170/0x170 [ 7552.800151] [<ffffffffb76dd1d8>] ret_from_fork+0x58/0x90 [ 7552.800153] [<ffffffffb70ac560>] ? kthread_worker_fn+0x170/0x170 [ 7552.800154] ---[ end trace 0000000000000002 ]--- [ 7554.164964] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.231254] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.294860] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.360810] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.421867] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.485931] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.546909] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.607820] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.671883] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. [ 7554.730826] srpt/0xf4521403000e4aa0f4521403000e4ad0: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION. static int __transport_check_aborted_status(struct se_cmd *cmd, int send_status) __releases(&cmd->t_state_lock) __acquires(&cmd->t_state_lock) { int ret; assert_spin_locked(&cmd->t_state_lock); WARN_ON_ONCE_NONRT(!irqs_disabled()); <SNIP> And it is called just from these two places: int transport_check_aborted_status(struct se_cmd *cmd, int send_status) { int ret; spin_lock_irq(&cmd->t_state_lock); ret = __transport_check_aborted_status(cmd, send_status); spin_unlock_irq(&cmd->t_state_lock); return ret; } EXPORT_SYMBOL(transport_check_aborted_status); And: void target_execute_cmd(struct se_cmd *cmd) { /* * Determine if frontend context caller is requesting the stopping of * this command for frontend exceptions. * * If the received CDB has aleady been aborted stop processing it here. */ spin_lock_irq(&cmd->t_state_lock); if (__transport_check_aborted_status(cmd, 1)) { spin_unlock_irq(&cmd->t_state_lock); return; } <SNIP> Since cmd->t_state_lock becomes a sleeping spin lock on RT, that thing triggers, turn it into a NONRT WARN_ON, a kernel built with this patch passes the test case that lead to a BZ being filled for the kernel-rt package: https://bugzilla.redhat.com/show_bug.cgi?id=1512875 Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html