Re: [PATCH] IB/ehca: Fix lockdep failures for shca_list_lock

[Michael Ellerman]

On Fri, 2008-11-21 at 17:02 +0100, Johannes Berg wrote:
> On Fri, 2008-11-21 at 16:37 +0100, Joachim Fenkes wrote:
> 
> > +   u64 flags;
> 
> > -   spin_lock(&shca_list_lock);
> > +   spin_lock_irqsave(&shca_list_lock, flags);
> 
> That's wrong and I think will give a warning on all machines where
> u64 != unsigned long. Might not particularly matter in this case.

Crud, sorry.

> Also, generally it seems wrong to say "fix lockdep failure" when the
> patch really fixes a bug that lockdep happened to find.

True. I guess it should be "fix locking error found with lockdep", to
make it clear no one has actually hit the bug.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] IB/ehca: Fix lockdep failures for shca_list_lock

[Johannes Berg]

On Fri, 2008-11-21 at 16:37 +0100, Joachim Fenkes wrote:

> + u64 flags;

> - spin_lock(&shca_list_lock);
> + spin_lock_irqsave(&shca_list_lock, flags);

That's wrong and I think will give a warning on all machines where
u64 != unsigned long. Might not particularly matter in this case.

Also, generally it seems wrong to say "fix lockdep failure" when the
patch really fixes a bug that lockdep happened to find.

johannes


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] IB/ehca: Fix lockdep failures for shca_list_lock

[Joachim Fenkes]

From: Michael Ellerman <[EMAIL PROTECTED]>

shca_list_lock is taken from softirq context in ehca_poll_eqs, so we need to
lock IRQ safe elsewhere.

Signed-off-by: Michael Ellerman <[EMAIL PROTECTED]>
Acked-by: Joachim Fenkes <[EMAIL PROTECTED]>
---
 drivers/infiniband/hw/ehca/ehca_main.c |   17 ++---
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_main.c 
b/drivers/infiniband/hw/ehca/ehca_main.c
index bb02a86..021c454 100644
--- a/drivers/infiniband/hw/ehca/ehca_main.c
+++ b/drivers/infiniband/hw/ehca/ehca_main.c
@@ -717,6 +717,7 @@ static int __devinit ehca_probe(struct of_device *dev,
const u64 *handle;
struct ib_pd *ibpd;
int ret, i, eq_size;
+   u64 flags;
 
handle = of_get_property(dev->node, "ibm,hca-handle", NULL);
if (!handle) {
@@ -830,9 +831,9 @@ static int __devinit ehca_probe(struct of_device *dev,
ehca_err(&shca->ib_device,
 "Cannot create device attributes  ret=%d", ret);
 
-   spin_lock(&shca_list_lock);
+   spin_lock_irqsave(&shca_list_lock, flags);
list_add(&shca->shca_list, &shca_list);
-   spin_unlock(&shca_list_lock);
+   spin_unlock_irqrestore(&shca_list_lock, flags);
 
return 0;
 
@@ -878,6 +879,7 @@ probe1:
 static int __devexit ehca_remove(struct of_device *dev)
 {
struct ehca_shca *shca = dev->dev.driver_data;
+   u64 flags;
int ret;
 
sysfs_remove_group(&dev->dev.kobj, &ehca_dev_attr_grp);
@@ -915,9 +917,9 @@ static int __devexit ehca_remove(struct of_device *dev)
 
ib_dealloc_device(&shca->ib_device);
 
-   spin_lock(&shca_list_lock);
+   spin_lock_irqsave(&shca_list_lock, flags);
list_del(&shca->shca_list);
-   spin_unlock(&shca_list_lock);
+   spin_unlock_irqrestore(&shca_list_lock, flags);
 
return ret;
 }
@@ -975,6 +977,7 @@ static int ehca_mem_notifier(struct notifier_block *nb,
 unsigned long action, void *data)
 {
static unsigned long ehca_dmem_warn_time;
+   unsigned long flags;
 
switch (action) {
case MEM_CANCEL_OFFLINE:
@@ -985,12 +988,12 @@ static int ehca_mem_notifier(struct notifier_block *nb,
case MEM_GOING_ONLINE:
case MEM_GOING_OFFLINE:
/* only ok if no hca is attached to the lpar */
-   spin_lock(&shca_list_lock);
+   spin_lock_irqsave(&shca_list_lock, flags);
if (list_empty(&shca_list)) {
-   spin_unlock(&shca_list_lock);
+   spin_unlock_irqrestore(&shca_list_lock, flags);
return NOTIFY_OK;
} else {
-   spin_unlock(&shca_list_lock);
+   spin_unlock_irqrestore(&shca_list_lock, flags);
if (printk_timed_ratelimit(&ehca_dmem_warn_time,
   30 * 1000))
ehca_gen_err("DMEM operations are not allowed"
-- 
1.5.5



___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev