public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org,
	linux-rt-users <linux-rt-users@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Carsten Emde <C.Emde@osadl.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	John Kacur <jkacur@redhat.com>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	<stable-rt@vger.kernel.org>
Subject: [PATCH RT 24/36] sas-ata/isci: dontt disable interrupts in qc_issue handler
Date: Thu, 12 Mar 2015 15:22:03 -0400	[thread overview]
Message-ID: <20150312192158.718272881@goodmis.org> (raw)
In-Reply-To: 20150312192139.799127123@goodmis.org

[-- Attachment #1: 0024-sas-ata-isci-dont-t-disable-interrupts-in-qc_issue-h.patch --]
[-- Type: text/plain, Size: 3473 bytes --]

3.12.38-rt53-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Paul Gortmaker <paul.gortmaker@windriver.com>

On 3.14-rt we see the following trace on Canoe Pass for
SCSI_ISCI "Intel(R) C600 Series Chipset SAS Controller"
when the sas qc_issue handler is run:

 BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:905
 in_atomic(): 0, irqs_disabled(): 1, pid: 432, name: udevd
 CPU: 11 PID: 432 Comm: udevd Not tainted 3.14.28-rt22 #2
 Hardware name: Intel Corporation S2600CP/S2600CP, BIOS SE5C600.86B.02.01.0002.082220131453 08/22/2013
 ffff880fab500000 ffff880fa9f239c0 ffffffff81a2d273 0000000000000000
 ffff880fa9f239d8 ffffffff8107f023 ffff880faac23dc0 ffff880fa9f239f0
 ffffffff81a33cc0 ffff880faaeb1400 ffff880fa9f23a40 ffffffff815de891
 Call Trace:
 [<ffffffff81a2d273>] dump_stack+0x4e/0x7a
 [<ffffffff8107f023>] __might_sleep+0xe3/0x160
 [<ffffffff81a33cc0>] rt_spin_lock+0x20/0x50
 [<ffffffff815de891>] isci_task_execute_task+0x171/0x2f0  <-----
 [<ffffffff815cfecb>] sas_ata_qc_issue+0x25b/0x2a0
 [<ffffffff81606363>] ata_qc_issue+0x1f3/0x370
 [<ffffffff8160c600>] ? ata_scsi_invalid_field+0x40/0x40
 [<ffffffff8160c8f5>] ata_scsi_translate+0xa5/0x1b0
 [<ffffffff8160efc6>] ata_sas_queuecmd+0x86/0x280
 [<ffffffff815ce446>] sas_queuecommand+0x196/0x230
 [<ffffffff81081fad>] ? get_parent_ip+0xd/0x50
 [<ffffffff815b05a4>] scsi_dispatch_cmd+0xb4/0x210
 [<ffffffff815b7744>] scsi_request_fn+0x314/0x530

and gdb shows:

(gdb) list * isci_task_execute_task+0x171
0xffffffff815ddfb1 is in isci_task_execute_task (drivers/scsi/isci/task.c:138).
133             dev_dbg(&ihost->pdev->dev, "%s: num=%d\n", __func__, num);
134
135             for_each_sas_task(num, task) {
136                     enum sci_status status = SCI_FAILURE;
137
138                     spin_lock_irqsave(&ihost->scic_lock, flags);    <-----
139                     idev = isci_lookup_device(task->dev);
140                     io_ready = isci_device_io_ready(idev, task);
141                     tag = isci_alloc_tag(ihost);
142                     spin_unlock_irqrestore(&ihost->scic_lock, flags);
(gdb)

In addition to the scic_lock, the function also contains locking of
the task_state_lock -- which is clearly not a candidate for raw lock
conversion.  As can be seen by the comment nearby, we really should
be running the qc_issue code with interrupts enabled anyway.

Cc: stable-rt@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 drivers/scsi/libsas/sas_ata.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index d2895836f9fa..c85e07ab51e4 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -191,7 +191,7 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
 	/* TODO: audit callers to ensure they are ready for qc_issue to
 	 * unconditionally re-enable interrupts
 	 */
-	local_irq_save(flags);
+	local_irq_save_nort(flags);
 	spin_unlock(ap->lock);
 
 	/* If the device fell off, no sense in issuing commands */
@@ -261,7 +261,7 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
 
  out:
 	spin_lock(ap->lock);
-	local_irq_restore(flags);
+	local_irq_restore_nort(flags);
 	return ret;
 }
 
-- 
2.1.4



  parent reply	other threads:[~2015-03-12 19:27 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-12 19:21 [PATCH RT 00/36] Linux 3.12.38-rt53-rc1 Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 01/36] gpio: omap: use raw locks for locking Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 02/36] create-rt-enqueue Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 03/36] rtmutex: Simplify rtmutex_slowtrylock() Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 04/36] rtmutex: Simplify and document try_to_take_rtmutex() Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 05/36] rtmutex: No need to keep task ref for lock owner check Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 06/36] rtmutex: Clarify the boost/deboost part Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 07/36] rtmutex: Document pi chain walk Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 08/36] rtmutex: Simplify remove_waiter() Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 09/36] rtmutex: Confine deadlock logic to futex Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 10/36] rtmutex: Cleanup deadlock detector debug logic Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 11/36] rtmutex: Avoid pointless requeueing in the deadlock detection chain walk Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 12/36] futex: Make unlock_pi more robust Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 13/36] futex: Use futex_top_waiter() in lookup_pi_state() Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 14/36] futex: Split out the waiter check from lookup_pi_state() Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 15/36] futex: Split out the first waiter attachment " Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 16/36] futex: Simplify futex_lock_pi_atomic() and make it more robust Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 17/36] rt-mutex: avoid a NULL pointer dereference on deadlock Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 18/36] rt: fix __ww_mutex_lock_interruptible() lockdep annotation Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 19/36] rtmutex: enable deadlock detection in ww_mutex_lock functions Steven Rostedt
2015-03-12 19:21 ` [PATCH RT 20/36] x86: UV: raw_spinlock conversion Steven Rostedt
2015-03-13 15:13   ` [PATCH RT 21/36] ARM: enable irq in translation/section permission fault handlers Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 22/36] arm/futex: disable preemption during futex_atomic_cmpxchg_inatomic() Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 23/36] ARM: cmpxchg: define __HAVE_ARCH_CMPXCHG for armv6 and later Steven Rostedt
2015-03-12 19:22 ` Steven Rostedt [this message]
2015-03-12 19:22 ` [PATCH RT 25/36] scheduling while atomic in cgroup code Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 26/36] work-simple: Simple work queue implemenation Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 27/36] sunrpc: make svc_xprt_do_enqueue() use get_cpu_light() Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 28/36] locking: ww_mutex: fix ww_mutex vs self-deadlock Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 29/36] thermal: Defer thermal wakups to threads Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 30/36] lockdep: selftest: fix warnings due to missing PREEMPT_RT conditionals Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 31/36] fs/aio: simple simple work Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 32/36] timers: Track total number of timers in list Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 33/36] timers: Reduce __run_timers() latency for empty list Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 34/36] timers: Reduce future __run_timers() latency for newly emptied list Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 35/36] timers: Reduce future __run_timers() latency for first add to empty list Steven Rostedt
2015-03-12 19:22 ` [PATCH RT 36/36] Linux 3.12.38-rt53-rc1 Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150312192158.718272881@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=C.Emde@osadl.org \
    --cc=bigeasy@linutronix.de \
    --cc=jkacur@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=paul.gortmaker@windriver.com \
    --cc=stable-rt@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox