linux-rt-users.vger.kernel.org archive mirror
 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 25/39] sas-ata/isci: dontt disable interrupts in qc_issue handler
Date: Thu, 12 Mar 2015 15:13:32 -0400	[thread overview]
Message-ID: <20150312191411.207321260@goodmis.org> (raw)
In-Reply-To: 20150312191307.081068717@goodmis.org

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

3.14.34-rt32-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:14 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-12 19:13 [PATCH RT 00/39] Linux 3.14.34-rt32-rc1 Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 01/39] gpio: omap: use raw locks for locking Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 02/39] rtmutex: Simplify rtmutex_slowtrylock() Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 03/39] rtmutex: Simplify and document try_to_take_rtmutex() Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 04/39] rtmutex: No need to keep task ref for lock owner check Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 05/39] rtmutex: Clarify the boost/deboost part Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 06/39] rtmutex: Document pi chain walk Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 07/39] rtmutex: Simplify remove_waiter() Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 08/39] rtmutex: Confine deadlock logic to futex Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 09/39] rtmutex: Cleanup deadlock detector debug logic Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 10/39] rtmutex: Avoid pointless requeueing in the deadlock detection chain walk Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 11/39] futex: Make unlock_pi more robust Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 12/39] futex: Use futex_top_waiter() in lookup_pi_state() Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 13/39] futex: Split out the waiter check from lookup_pi_state() Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 14/39] futex: Split out the first waiter attachment " Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 15/39] futex: Simplify futex_lock_pi_atomic() and make it more robust Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 16/39] locking/rt-mutex: avoid a NULL pointer dereference on deadlock Steven Rostedt
2015-03-13 10:40   ` Sebastian Andrzej Siewior
2015-03-13 10:56     ` Sebastian Andrzej Siewior
2015-03-12 19:13 ` [PATCH RT 17/39] rtmutex.c: Fix incorrect waiter check Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 18/39] rt,locking: fix __ww_mutex_lock_interruptible() lockdep annotation Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 19/39] rtmutex: enable deadlock detection in ww_mutex_lock functions Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 20/39] x86: UV: raw_spinlock conversion Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 22/39] arm/futex: disable preemption during futex_atomic_cmpxchg_inatomic() Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 23/39] ARM: cmpxchg: define __HAVE_ARCH_CMPXCHG for armv6 and later Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 24/39] mips: rt: Replace pagefault_* to raw version Steven Rostedt
2015-03-12 19:13 ` Steven Rostedt [this message]
2015-03-12 19:13 ` [PATCH RT 26/39] scheduling while atomic in cgroup code Steven Rostedt
2015-03-17 20:10   ` Paul Gortmaker
2015-03-17 20:13     ` Steven Rostedt
2015-03-18  8:37     ` Sebastian Andrzej Siewior
2015-03-18 13:20       ` Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 27/39] work-simple: Simple work queue implemenation Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 28/39] sunrpc: make svc_xprt_do_enqueue() use get_cpu_light() Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 29/39] Revert "rwsem-rt: Do not allow readers to nest" Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 30/39] locking: ww_mutex: fix ww_mutex vs self-deadlock Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 31/39] thermal: Defer thermal wakups to threads Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 32/39] lockdep: selftest: fix warnings due to missing PREEMPT_RT conditionals Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 33/39] fs/aio: simple simple work Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 34/39] timers: Track total number of timers in list Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 35/39] timers: Reduce __run_timers() latency for empty list Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 36/39] timers: Reduce future __run_timers() latency for newly emptied list Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 37/39] timers: Reduce future __run_timers() latency for first add to empty list Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 38/39] staging: Mark rtl8821ae as broken Steven Rostedt
2015-03-12 19:13 ` [PATCH RT 39/39] Linux 3.14.34-rt32-rc1 Steven Rostedt
2015-03-13  4:49 ` [PATCH RT 00/39] " Mike Galbraith
2015-03-13 13:50   ` Steven Rostedt
2015-03-13 15:11   ` Steven Rostedt
2015-03-13 15:24     ` Steven Rostedt
2015-03-13 11:01 ` Sebastian Andrzej Siewior
2015-03-13 11:33 ` Sebastian Andrzej Siewior
2015-03-16 13:59 ` Sebastian Andrzej Siewior
2015-03-16 14:02   ` Steven Rostedt
2015-03-16 14:10     ` Sebastian Andrzej Siewior

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=20150312191411.207321260@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;
as well as URLs for NNTP newsgroup(s).