* [PATCH] libsas: fix lockdep issue with ATA
@ 2007-07-16 18:15 James Bottomley
2007-07-17 11:29 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2007-07-16 18:15 UTC (permalink / raw)
To: linux-scsi
lockdep noticed that with ATA support the port->dev_list_lock was
entangled at irq context, so it now needs to become IRQ safe
---
This applies against the aic94xx-sas-2.6 tree
James
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index a18c0f6..ce3385c 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -292,9 +292,9 @@ static int sas_get_port_device(struct asd_sas_port *port)
port->disc.max_level = 0;
dev->rphy = rphy;
- spin_lock(&port->dev_list_lock);
+ spin_lock_irqsave(&port->dev_list_lock, flags);
list_add_tail(&dev->dev_list_node, &port->dev_list);
- spin_unlock(&port->dev_list_lock);
+ spin_unlock_irqrestore(&port->dev_list_lock, flags);
return 0;
}
@@ -688,12 +688,14 @@ static void sas_discover_domain(struct work_struct *work)
}
if (error) {
+ unsigned long flags;
+
sas_rphy_free(dev->rphy);
dev->rphy = NULL;
- spin_lock(&port->dev_list_lock);
+ spin_lock_irqsave(&port->dev_list_lock, flags);
list_del_init(&dev->dev_list_node);
- spin_unlock(&port->dev_list_lock);
+ spin_unlock_irqrestore(&port->dev_list_lock, flags);
kfree(dev); /* not kobject_register-ed yet */
port->port_dev = NULL;
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index d05fc23..1c15f37 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -628,6 +628,7 @@ static struct domain_device *sas_ex_discover_end_dev(
struct domain_device *child = NULL;
struct sas_rphy *rphy;
int res;
+ unsigned long flags;
if (phy->attached_sata_host || phy->attached_sata_ps)
return NULL;
@@ -677,9 +678,9 @@ static struct domain_device *sas_ex_discover_end_dev(
child->rphy = rphy;
- spin_lock(&parent->port->dev_list_lock);
+ spin_lock_irqsave(&parent->port->dev_list_lock, flags);
list_add_tail(&child->dev_list_node, &parent->port->dev_list);
- spin_unlock(&parent->port->dev_list_lock);
+ spin_unlock_irqrestore(&parent->port->dev_list_lock, flags);
res = sas_discover_sata(child);
if (res) {
@@ -701,9 +702,9 @@ static struct domain_device *sas_ex_discover_end_dev(
child->rphy = rphy;
sas_fill_in_rphy(child, rphy);
- spin_lock(&parent->port->dev_list_lock);
+ spin_lock_irqsave(&parent->port->dev_list_lock, flags);
list_add_tail(&child->dev_list_node, &parent->port->dev_list);
- spin_unlock(&parent->port->dev_list_lock);
+ spin_unlock_irqrestore(&parent->port->dev_list_lock, flags);
res = sas_discover_end_dev(child);
if (res) {
@@ -767,6 +768,7 @@ static struct domain_device *sas_ex_discover_expander(
struct sas_expander_device *edev;
struct asd_sas_port *port;
int res;
+ unsigned long flags;
if (phy->routing_attr == DIRECT_ROUTING) {
SAS_DPRINTK("ex %016llx:0x%x:D <--> ex %016llx:0x%x is not "
@@ -816,9 +818,9 @@ static struct domain_device *sas_ex_discover_expander(
sas_fill_in_rphy(child, rphy);
sas_rphy_add(rphy);
- spin_lock(&parent->port->dev_list_lock);
+ spin_lock_irqsave(&parent->port->dev_list_lock, flags);
list_add_tail(&child->dev_list_node, &parent->port->dev_list);
- spin_unlock(&parent->port->dev_list_lock);
+ spin_unlock_irqrestore(&parent->port->dev_list_lock, flags);
res = sas_discover_expander(child);
if (res) {
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] libsas: fix lockdep issue with ATA
2007-07-16 18:15 [PATCH] libsas: fix lockdep issue with ATA James Bottomley
@ 2007-07-17 11:29 ` Jens Axboe
2007-07-17 14:12 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2007-07-17 11:29 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-scsi
On Mon, Jul 16 2007, James Bottomley wrote:
> lockdep noticed that with ATA support the port->dev_list_lock was
> entangled at irq context, so it now needs to become IRQ safe
>
> ---
> This applies against the aic94xx-sas-2.6 tree
>
> James
>
> diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
> index a18c0f6..ce3385c 100644
> --- a/drivers/scsi/libsas/sas_discover.c
> +++ b/drivers/scsi/libsas/sas_discover.c
> @@ -292,9 +292,9 @@ static int sas_get_port_device(struct asd_sas_port *port)
> port->disc.max_level = 0;
>
> dev->rphy = rphy;
> - spin_lock(&port->dev_list_lock);
> + spin_lock_irqsave(&port->dev_list_lock, flags);
> list_add_tail(&dev->dev_list_node, &port->dev_list);
> - spin_unlock(&port->dev_list_lock);
> + spin_unlock_irqrestore(&port->dev_list_lock, flags);
>
> return 0;
> }
already uses GFP_KERNEL, so spin_lock_irq() suffices.
> @@ -688,12 +688,14 @@ static void sas_discover_domain(struct work_struct *work)
> }
>
> if (error) {
> + unsigned long flags;
> +
> sas_rphy_free(dev->rphy);
> dev->rphy = NULL;
>
> - spin_lock(&port->dev_list_lock);
> + spin_lock_irqsave(&port->dev_list_lock, flags);
> list_del_init(&dev->dev_list_node);
> - spin_unlock(&port->dev_list_lock);
> + spin_unlock_irqrestore(&port->dev_list_lock, flags);
>
> kfree(dev); /* not kobject_register-ed yet */
> port->port_dev = NULL;
process context, no need to save flags here either.
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index d05fc23..1c15f37 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -628,6 +628,7 @@ static struct domain_device *sas_ex_discover_end_dev(
> struct domain_device *child = NULL;
> struct sas_rphy *rphy;
> int res;
> + unsigned long flags;
>
> if (phy->attached_sata_host || phy->attached_sata_ps)
> return NULL;
> @@ -677,9 +678,9 @@ static struct domain_device *sas_ex_discover_end_dev(
>
> child->rphy = rphy;
>
> - spin_lock(&parent->port->dev_list_lock);
> + spin_lock_irqsave(&parent->port->dev_list_lock, flags);
> list_add_tail(&child->dev_list_node, &parent->port->dev_list);
> - spin_unlock(&parent->port->dev_list_lock);
> + spin_unlock_irqrestore(&parent->port->dev_list_lock, flags);
>
> res = sas_discover_sata(child);
> if (res) {
ditto
> @@ -701,9 +702,9 @@ static struct domain_device *sas_ex_discover_end_dev(
> child->rphy = rphy;
> sas_fill_in_rphy(child, rphy);
>
> - spin_lock(&parent->port->dev_list_lock);
> + spin_lock_irqsave(&parent->port->dev_list_lock, flags);
> list_add_tail(&child->dev_list_node, &parent->port->dev_list);
> - spin_unlock(&parent->port->dev_list_lock);
> + spin_unlock_irqrestore(&parent->port->dev_list_lock, flags);
>
> res = sas_discover_end_dev(child);
> if (res) {
ditto
> @@ -767,6 +768,7 @@ static struct domain_device *sas_ex_discover_expander(
> struct sas_expander_device *edev;
> struct asd_sas_port *port;
> int res;
> + unsigned long flags;
>
> if (phy->routing_attr == DIRECT_ROUTING) {
> SAS_DPRINTK("ex %016llx:0x%x:D <--> ex %016llx:0x%x is not "
> @@ -816,9 +818,9 @@ static struct domain_device *sas_ex_discover_expander(
> sas_fill_in_rphy(child, rphy);
> sas_rphy_add(rphy);
>
> - spin_lock(&parent->port->dev_list_lock);
> + spin_lock_irqsave(&parent->port->dev_list_lock, flags);
> list_add_tail(&child->dev_list_node, &parent->port->dev_list);
> - spin_unlock(&parent->port->dev_list_lock);
> + spin_unlock_irqrestore(&parent->port->dev_list_lock, flags);
>
> res = sas_discover_expander(child);
> if (res) {
ditto :-)
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] libsas: fix lockdep issue with ATA
2007-07-17 11:29 ` Jens Axboe
@ 2007-07-17 14:12 ` James Bottomley
2007-07-17 16:35 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2007-07-17 14:12 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-scsi
On Tue, 2007-07-17 at 13:29 +0200, Jens Axboe wrote:
> On Mon, Jul 16 2007, James Bottomley wrote:
> > lockdep noticed that with ATA support the port->dev_list_lock was
> > entangled at irq context, so it now needs to become IRQ safe
> >
> > ---
> > This applies against the aic94xx-sas-2.6 tree
> >
> > James
> >
> > diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
> > index a18c0f6..ce3385c 100644
> > --- a/drivers/scsi/libsas/sas_discover.c
> > +++ b/drivers/scsi/libsas/sas_discover.c
> > @@ -292,9 +292,9 @@ static int sas_get_port_device(struct asd_sas_port *port)
> > port->disc.max_level = 0;
> >
> > dev->rphy = rphy;
> > - spin_lock(&port->dev_list_lock);
> > + spin_lock_irqsave(&port->dev_list_lock, flags);
> > list_add_tail(&dev->dev_list_node, &port->dev_list);
> > - spin_unlock(&port->dev_list_lock);
> > + spin_unlock_irqrestore(&port->dev_list_lock, flags);
> >
> > return 0;
> > }
>
> already uses GFP_KERNEL, so spin_lock_irq() suffices.
>
> > @@ -688,12 +688,14 @@ static void sas_discover_domain(struct work_struct *work)
> > }
> >
> > if (error) {
> > + unsigned long flags;
> > +
> > sas_rphy_free(dev->rphy);
> > dev->rphy = NULL;
> >
> > - spin_lock(&port->dev_list_lock);
> > + spin_lock_irqsave(&port->dev_list_lock, flags);
> > list_del_init(&dev->dev_list_node);
> > - spin_unlock(&port->dev_list_lock);
> > + spin_unlock_irqrestore(&port->dev_list_lock, flags);
> >
> > kfree(dev); /* not kobject_register-ed yet */
> > port->port_dev = NULL;
>
> process context, no need to save flags here either.
OK, you caught me ... I was just doing the fastest thing I could to get
lockdep to shut up. By the way, it dumps about 120KB of logs so it's
fun to get an actual trace of this...
I'll make the fixes and resubmit
James
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] libsas: fix lockdep issue with ATA
2007-07-17 14:12 ` James Bottomley
@ 2007-07-17 16:35 ` James Bottomley
2007-07-18 8:10 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2007-07-17 16:35 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-scsi
On Tue, 2007-07-17 at 09:12 -0500, James Bottomley wrote:
> > process context, no need to save flags here either.
>
> OK, you caught me ... I was just doing the fastest thing I could to get
> lockdep to shut up. By the way, it dumps about 120KB of logs so it's
> fun to get an actual trace of this...
>
> I'll make the fixes and resubmit
Here's the update.
James
---
>From 0852ea7aafd0368026e8dacd7810c2444adb8d19 Mon Sep 17 00:00:00 2001
From: James Bottomley <James.Bottomley@steeleye.com>
Date: Mon, 16 Jul 2007 13:15:51 -0500
Subject: [SCSI] libsas: fix lockdep issue with ATA
lockdep noticed that with ATA support the port->dev_list_lock was
entangled at irq context, so it now needs to become IRQ safe
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
---
drivers/scsi/libsas/sas_discover.c | 10 ++++++----
drivers/scsi/libsas/sas_expander.c | 14 ++++++++------
2 files changed, 14 insertions(+), 10 deletions(-)
Index: BUILD-2.6/drivers/scsi/libsas/sas_discover.c
===================================================================
--- BUILD-2.6.orig/drivers/scsi/libsas/sas_discover.c 2007-07-17 11:15:48.000000000 -0500
+++ BUILD-2.6/drivers/scsi/libsas/sas_discover.c 2007-07-17 11:26:47.000000000 -0500
@@ -304,9 +304,9 @@ static int sas_get_port_device(struct as
port->disc.max_level = 0;
dev->rphy = rphy;
- spin_lock(&port->dev_list_lock);
+ spin_lock_irq(&port->dev_list_lock);
list_add_tail(&dev->dev_list_node, &port->dev_list);
- spin_unlock(&port->dev_list_lock);
+ spin_unlock_irq(&port->dev_list_lock);
return 0;
}
@@ -703,9 +703,9 @@ static void sas_discover_domain(struct w
sas_rphy_free(dev->rphy);
dev->rphy = NULL;
- spin_lock(&port->dev_list_lock);
+ spin_lock_irq(&port->dev_list_lock);
list_del_init(&dev->dev_list_node);
- spin_unlock(&port->dev_list_lock);
+ spin_unlock_irq(&port->dev_list_lock);
kfree(dev); /* not kobject_register-ed yet */
port->port_dev = NULL;
Index: BUILD-2.6/drivers/scsi/libsas/sas_expander.c
===================================================================
--- BUILD-2.6.orig/drivers/scsi/libsas/sas_expander.c 2007-07-17 11:15:48.000000000 -0500
+++ BUILD-2.6/drivers/scsi/libsas/sas_expander.c 2007-07-17 11:28:33.000000000 -0500
@@ -677,9 +677,9 @@ static struct domain_device *sas_ex_disc
child->rphy = rphy;
- spin_lock(&parent->port->dev_list_lock);
+ spin_lock_irq(&parent->port->dev_list_lock);
list_add_tail(&child->dev_list_node, &parent->port->dev_list);
- spin_unlock(&parent->port->dev_list_lock);
+ spin_unlock_irq(&parent->port->dev_list_lock);
res = sas_discover_sata(child);
if (res) {
@@ -701,9 +701,9 @@ static struct domain_device *sas_ex_disc
child->rphy = rphy;
sas_fill_in_rphy(child, rphy);
- spin_lock(&parent->port->dev_list_lock);
+ spin_lock_irq(&parent->port->dev_list_lock);
list_add_tail(&child->dev_list_node, &parent->port->dev_list);
- spin_unlock(&parent->port->dev_list_lock);
+ spin_unlock_irq(&parent->port->dev_list_lock);
res = sas_discover_end_dev(child);
if (res) {
@@ -816,9 +816,9 @@ static struct domain_device *sas_ex_disc
sas_fill_in_rphy(child, rphy);
sas_rphy_add(rphy);
- spin_lock(&parent->port->dev_list_lock);
+ spin_lock_irq(&parent->port->dev_list_lock);
list_add_tail(&child->dev_list_node, &parent->port->dev_list);
- spin_unlock(&parent->port->dev_list_lock);
+ spin_unlock_irq(&parent->port->dev_list_lock);
res = sas_discover_expander(child);
if (res) {
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] libsas: fix lockdep issue with ATA
2007-07-17 16:35 ` James Bottomley
@ 2007-07-18 8:10 ` Jens Axboe
0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2007-07-18 8:10 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-scsi
On Tue, Jul 17 2007, James Bottomley wrote:
> On Tue, 2007-07-17 at 09:12 -0500, James Bottomley wrote:
> > > process context, no need to save flags here either.
> >
> > OK, you caught me ... I was just doing the fastest thing I could to get
> > lockdep to shut up. By the way, it dumps about 120KB of logs so it's
> > fun to get an actual trace of this...
> >
> > I'll make the fixes and resubmit
>
> Here's the update.
Looks good James :-)
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-07-18 8:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-16 18:15 [PATCH] libsas: fix lockdep issue with ATA James Bottomley
2007-07-17 11:29 ` Jens Axboe
2007-07-17 14:12 ` James Bottomley
2007-07-17 16:35 ` James Bottomley
2007-07-18 8:10 ` Jens Axboe
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).