* [patch] sgi-xp: nested calls to spin_lock_irqsave()
@ 2011-12-15 6:48 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2011-12-15 6:48 UTC (permalink / raw)
To: Robin Holt; +Cc: linux-kernel, kernel-janitors
The code here has a nested spin_lock_irqsave(). It's not needed since
IRQs are already disabled and it causes a problem because it means that
IRQs won't be enabled again at the end. The second call to
spin_lock_irqsave() will overwrite the value of irq_flags and we can't
restore the proper settings.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is a static checker fix. It's possible that no one hits this bug
in real life if the function is always called with IRQs disabled.
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 17bbacb..87b251a 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -452,9 +452,9 @@ xpc_handle_activate_mq_msg_uv(struct xpc_partition *part,
if (msg->activate_gru_mq_desc_gpa !=
part_uv->activate_gru_mq_desc_gpa) {
- spin_lock_irqsave(&part_uv->flags_lock, irq_flags);
+ spin_lock(&part_uv->flags_lock);
part_uv->flags &= ~XPC_P_CACHED_ACTIVATE_GRU_MQ_DESC_UV;
- spin_unlock_irqrestore(&part_uv->flags_lock, irq_flags);
+ spin_unlock(&part_uv->flags_lock);
part_uv->activate_gru_mq_desc_gpa =
msg->activate_gru_mq_desc_gpa;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [patch] sgi-xp: nested calls to spin_lock_irqsave()
@ 2011-12-15 6:48 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2011-12-15 6:48 UTC (permalink / raw)
To: Robin Holt; +Cc: linux-kernel, kernel-janitors
The code here has a nested spin_lock_irqsave(). It's not needed since
IRQs are already disabled and it causes a problem because it means that
IRQs won't be enabled again at the end. The second call to
spin_lock_irqsave() will overwrite the value of irq_flags and we can't
restore the proper settings.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is a static checker fix. It's possible that no one hits this bug
in real life if the function is always called with IRQs disabled.
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 17bbacb..87b251a 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -452,9 +452,9 @@ xpc_handle_activate_mq_msg_uv(struct xpc_partition *part,
if (msg->activate_gru_mq_desc_gpa ! part_uv->activate_gru_mq_desc_gpa) {
- spin_lock_irqsave(&part_uv->flags_lock, irq_flags);
+ spin_lock(&part_uv->flags_lock);
part_uv->flags &= ~XPC_P_CACHED_ACTIVATE_GRU_MQ_DESC_UV;
- spin_unlock_irqrestore(&part_uv->flags_lock, irq_flags);
+ spin_unlock(&part_uv->flags_lock);
part_uv->activate_gru_mq_desc_gpa msg->activate_gru_mq_desc_gpa;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [patch] sgi-xp: nested calls to spin_lock_irqsave()
2011-12-15 6:48 ` Dan Carpenter
@ 2011-12-16 11:55 ` Robin Holt
-1 siblings, 0 replies; 10+ messages in thread
From: Robin Holt @ 2011-12-16 11:55 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Robin Holt, linux-kernel, kernel-janitors
On Thu, Dec 15, 2011 at 09:48:05AM +0300, Dan Carpenter wrote:
> The code here has a nested spin_lock_irqsave(). It's not needed since
> IRQs are already disabled and it causes a problem because it means that
> IRQs won't be enabled again at the end. The second call to
> spin_lock_irqsave() will overwrite the value of irq_flags and we can't
> restore the proper settings.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Robin Holt <holt@sgi.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] sgi-xp: nested calls to spin_lock_irqsave()
@ 2011-12-16 11:55 ` Robin Holt
0 siblings, 0 replies; 10+ messages in thread
From: Robin Holt @ 2011-12-16 11:55 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Robin Holt, linux-kernel, kernel-janitors
On Thu, Dec 15, 2011 at 09:48:05AM +0300, Dan Carpenter wrote:
> The code here has a nested spin_lock_irqsave(). It's not needed since
> IRQs are already disabled and it causes a problem because it means that
> IRQs won't be enabled again at the end. The second call to
> spin_lock_irqsave() will overwrite the value of irq_flags and we can't
> restore the proper settings.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Robin Holt <holt@sgi.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch] sgi-xp: nested calls to spin_lock_irqsave()
2011-12-15 6:48 ` Dan Carpenter
@ 2012-06-27 13:51 ` Robin Holt
-1 siblings, 0 replies; 10+ messages in thread
From: Robin Holt @ 2012-06-27 13:51 UTC (permalink / raw)
To: Andrew Morton
Cc: Robin Holt, Dan Carpenter, linux-kernel, kernel-janitors, Jack Steiner
The code here has a nested spin_lock_irqsave(). It's not needed since
IRQs are already disabled and it causes a problem because it means that
IRQs won't be enabled again at the end. The second call to
spin_lock_irqsave() will overwrite the value of irq_flags and we can't
restore the proper settings.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Robin Holt <holt@sgi.com>
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 17bbacb..87b251a 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -452,9 +452,9 @@ xpc_handle_activate_mq_msg_uv(struct xpc_partition *part,
if (msg->activate_gru_mq_desc_gpa !=
part_uv->activate_gru_mq_desc_gpa) {
- spin_lock_irqsave(&part_uv->flags_lock, irq_flags);
+ spin_lock(&part_uv->flags_lock);
part_uv->flags &= ~XPC_P_CACHED_ACTIVATE_GRU_MQ_DESC_UV;
- spin_unlock_irqrestore(&part_uv->flags_lock, irq_flags);
+ spin_unlock(&part_uv->flags_lock);
part_uv->activate_gru_mq_desc_gpa =
msg->activate_gru_mq_desc_gpa;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [patch] sgi-xp: nested calls to spin_lock_irqsave()
@ 2012-06-27 13:51 ` Robin Holt
0 siblings, 0 replies; 10+ messages in thread
From: Robin Holt @ 2012-06-27 13:51 UTC (permalink / raw)
To: Andrew Morton
Cc: Robin Holt, Dan Carpenter, linux-kernel, kernel-janitors, Jack Steiner
The code here has a nested spin_lock_irqsave(). It's not needed since
IRQs are already disabled and it causes a problem because it means that
IRQs won't be enabled again at the end. The second call to
spin_lock_irqsave() will overwrite the value of irq_flags and we can't
restore the proper settings.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Robin Holt <holt@sgi.com>
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 17bbacb..87b251a 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -452,9 +452,9 @@ xpc_handle_activate_mq_msg_uv(struct xpc_partition *part,
if (msg->activate_gru_mq_desc_gpa ! part_uv->activate_gru_mq_desc_gpa) {
- spin_lock_irqsave(&part_uv->flags_lock, irq_flags);
+ spin_lock(&part_uv->flags_lock);
part_uv->flags &= ~XPC_P_CACHED_ACTIVATE_GRU_MQ_DESC_UV;
- spin_unlock_irqrestore(&part_uv->flags_lock, irq_flags);
+ spin_unlock(&part_uv->flags_lock);
part_uv->activate_gru_mq_desc_gpa msg->activate_gru_mq_desc_gpa;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [patch] sgi-xp: nested calls to spin_lock_irqsave()
2012-06-27 13:51 ` Robin Holt
@ 2012-06-28 22:03 ` Andrew Morton
-1 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2012-06-28 22:03 UTC (permalink / raw)
To: Robin Holt; +Cc: Dan Carpenter, linux-kernel, kernel-janitors, Jack Steiner
On Wed, 27 Jun 2012 08:51:55 -0500
Robin Holt <holt@sgi.com> wrote:
> The code here has a nested spin_lock_irqsave(). It's not needed since
> IRQs are already disabled and it causes a problem because it means that
> IRQs won't be enabled again at the end. The second call to
> spin_lock_irqsave() will overwrite the value of irq_flags and we can't
> restore the proper settings.
Please find large crayon and write on forehead "when fixing a bug,
be sure to describe the end-user impact of that bug".
Then others will have a chance of deciding whether the fix should be
backported.
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Robin Holt <holt@sgi.com>
I'm wondering if there should have been a Reported-by:dan in there.
He's like that.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] sgi-xp: nested calls to spin_lock_irqsave()
@ 2012-06-28 22:03 ` Andrew Morton
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2012-06-28 22:03 UTC (permalink / raw)
To: Robin Holt; +Cc: Dan Carpenter, linux-kernel, kernel-janitors, Jack Steiner
On Wed, 27 Jun 2012 08:51:55 -0500
Robin Holt <holt@sgi.com> wrote:
> The code here has a nested spin_lock_irqsave(). It's not needed since
> IRQs are already disabled and it causes a problem because it means that
> IRQs won't be enabled again at the end. The second call to
> spin_lock_irqsave() will overwrite the value of irq_flags and we can't
> restore the proper settings.
Please find large crayon and write on forehead "when fixing a bug,
be sure to describe the end-user impact of that bug".
Then others will have a chance of deciding whether the fix should be
backported.
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Robin Holt <holt@sgi.com>
I'm wondering if there should have been a Reported-by:dan in there.
He's like that.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] sgi-xp: nested calls to spin_lock_irqsave()
2012-06-28 22:03 ` Andrew Morton
@ 2012-06-29 6:44 ` Dan Carpenter
-1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2012-06-29 6:44 UTC (permalink / raw)
To: Andrew Morton; +Cc: Robin Holt, linux-kernel, kernel-janitors, Jack Steiner
This is actually my patch, so the patch description is my fault.
It's a static checker patch and I don't have the hardware, but the
changelog could have been written more clearly as well.
Robin, the trick to resending patches is to put a:
From: Dan Carpenter <dan.carpenter@oracle.com>
before the patch description and then it shows up as mine.
Also, btw, Robin, I'm looking through my mailbox again and you did
respond to the patch the first time I sent it. Sorry, that I didn't
see that before, and thanks for your help on this.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] sgi-xp: nested calls to spin_lock_irqsave()
@ 2012-06-29 6:44 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2012-06-29 6:44 UTC (permalink / raw)
To: Andrew Morton; +Cc: Robin Holt, linux-kernel, kernel-janitors, Jack Steiner
This is actually my patch, so the patch description is my fault.
It's a static checker patch and I don't have the hardware, but the
changelog could have been written more clearly as well.
Robin, the trick to resending patches is to put a:
From: Dan Carpenter <dan.carpenter@oracle.com>
before the patch description and then it shows up as mine.
Also, btw, Robin, I'm looking through my mailbox again and you did
respond to the patch the first time I sent it. Sorry, that I didn't
see that before, and thanks for your help on this.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-06-29 6:44 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-27 13:51 [patch] sgi-xp: nested calls to spin_lock_irqsave() Robin Holt
2012-06-27 13:51 ` Robin Holt
2012-06-28 22:03 ` Andrew Morton
2012-06-28 22:03 ` Andrew Morton
2012-06-29 6:44 ` Dan Carpenter
2012-06-29 6:44 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2011-12-15 6:48 Dan Carpenter
2011-12-15 6:48 ` Dan Carpenter
2011-12-16 11:55 ` Robin Holt
2011-12-16 11:55 ` Robin Holt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.