linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure
@ 2010-11-16 16:32 Sergio Aguirre
  2010-11-16 16:45 ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Sergio Aguirre @ 2010-11-16 16:32 UTC (permalink / raw)
  To: LKML
  Cc: Sergio Aguirre, Huang Ying, Martin Schwidefsky, Ingo Molnar,
	Kyle McMartin, Peter Zijlstra

Although is very unlikely, it's better to make sure we're not
letting this happen.

This solves this compilation warning:

kernel/irq_work.c: In function 'irq_work_run':
kernel/irq_work.c:148: warning: value computed is not used

Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Kyle McMartin <kyle@mcmartin.ca>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 kernel/irq_work.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index f16763f..5da635b 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -131,7 +131,7 @@ void irq_work_run(void)
 
 	list = xchg(head, NULL);
 	while (list != NULL) {
-		struct irq_work *entry = list;
+		struct irq_work *entry = list, *xchgres;
 
 		list = irq_work_next(list);
 
@@ -145,7 +145,10 @@ void irq_work_run(void)
 		 * Clear the BUSY bit and return to the free state if
 		 * no-one else claimed it meanwhile.
 		 */
-		cmpxchg(&entry->next, next_flags(NULL, IRQ_WORK_BUSY), NULL);
+		xchgres = cmpxchg(&entry->next,
+				  next_flags(NULL, IRQ_WORK_BUSY),
+				  NULL);
+		BUG_ON(unlikely(xchgres != next_flags(NULL, IRQ_WORK_BUSY)));
 	}
 }
 EXPORT_SYMBOL_GPL(irq_work_run);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure
  2010-11-16 16:32 [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure Sergio Aguirre
@ 2010-11-16 16:45 ` Peter Zijlstra
  2010-11-16 16:57   ` Aguirre, Sergio
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2010-11-16 16:45 UTC (permalink / raw)
  To: Sergio Aguirre
  Cc: LKML, Huang Ying, Martin Schwidefsky, Ingo Molnar, Kyle McMartin

On Tue, 2010-11-16 at 10:32 -0600, Sergio Aguirre wrote:
> Although is very unlikely, it's better to make sure we're not
> letting this happen.
> 
> This solves this compilation warning:
> 
> kernel/irq_work.c: In function 'irq_work_run':
> kernel/irq_work.c:148: warning: value computed is not used
> 

> @@ -145,7 +145,10 @@ void irq_work_run(void)
>  		 * Clear the BUSY bit and return to the free state if
>  		 * no-one else claimed it meanwhile.
>  		 */
> -		cmpxchg(&entry->next, next_flags(NULL, IRQ_WORK_BUSY), NULL);
> +		xchgres = cmpxchg(&entry->next,
> +				  next_flags(NULL, IRQ_WORK_BUSY),
> +				  NULL);
> +		BUG_ON(unlikely(xchgres != next_flags(NULL, IRQ_WORK_BUSY)));

simply adding (void) in front would be much easier.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure
  2010-11-16 16:45 ` Peter Zijlstra
@ 2010-11-16 16:57   ` Aguirre, Sergio
  2010-11-16 17:08     ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Aguirre, Sergio @ 2010-11-16 16:57 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: LKML, Huang Ying, Martin Schwidefsky, Ingo Molnar, Kyle McMartin

Hi Peter,

> -----Original Message-----
> From: Peter Zijlstra [mailto:a.p.zijlstra@chello.nl]
> Sent: Tuesday, November 16, 2010 10:45 AM
> To: Aguirre, Sergio
> Cc: LKML; Huang Ying; Martin Schwidefsky; Ingo Molnar; Kyle McMartin
> Subject: Re: [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure
> 
> On Tue, 2010-11-16 at 10:32 -0600, Sergio Aguirre wrote:
> > Although is very unlikely, it's better to make sure we're not
> > letting this happen.
> >
> > This solves this compilation warning:
> >
> > kernel/irq_work.c: In function 'irq_work_run':
> > kernel/irq_work.c:148: warning: value computed is not used
> >
> 
> > @@ -145,7 +145,10 @@ void irq_work_run(void)
> >  		 * Clear the BUSY bit and return to the free state if
> >  		 * no-one else claimed it meanwhile.
> >  		 */
> > -		cmpxchg(&entry->next, next_flags(NULL, IRQ_WORK_BUSY), NULL);
> > +		xchgres = cmpxchg(&entry->next,
> > +				  next_flags(NULL, IRQ_WORK_BUSY),
> > +				  NULL);
> > +		BUG_ON(unlikely(xchgres != next_flags(NULL, IRQ_WORK_BUSY)));
> 
> simply adding (void) in front would be much easier.

But isn't that still leaving the remote possibility of a hidden cmpxchg
Failure open?

Regards,
Sergio

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure
  2010-11-16 16:57   ` Aguirre, Sergio
@ 2010-11-16 17:08     ` Peter Zijlstra
  2010-11-16 17:11       ` Aguirre, Sergio
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2010-11-16 17:08 UTC (permalink / raw)
  To: Aguirre, Sergio
  Cc: LKML, Huang Ying, Martin Schwidefsky, Ingo Molnar, Kyle McMartin

On Tue, 2010-11-16 at 10:57 -0600, Aguirre, Sergio wrote:

> > > @@ -145,7 +145,10 @@ void irq_work_run(void)
> > >  		 * Clear the BUSY bit and return to the free state if
> > >  		 * no-one else claimed it meanwhile.
> > >  		 */
> > > -		cmpxchg(&entry->next, next_flags(NULL, IRQ_WORK_BUSY), NULL);
> > > +		xchgres = cmpxchg(&entry->next,
> > > +				  next_flags(NULL, IRQ_WORK_BUSY),
> > > +				  NULL);
> > > +		BUG_ON(unlikely(xchgres != next_flags(NULL, IRQ_WORK_BUSY)));
> > 
> > simply adding (void) in front would be much easier.
> 
> But isn't that still leaving the remote possibility of a hidden cmpxchg
> Failure open?

No, we don't care if it fails, read the comment. All we want to know is
that if it still matched, we flipped the bit.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure
  2010-11-16 17:08     ` Peter Zijlstra
@ 2010-11-16 17:11       ` Aguirre, Sergio
  0 siblings, 0 replies; 5+ messages in thread
From: Aguirre, Sergio @ 2010-11-16 17:11 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: LKML, Huang Ying, Martin Schwidefsky, Ingo Molnar, Kyle McMartin



> -----Original Message-----
> From: Peter Zijlstra [mailto:a.p.zijlstra@chello.nl]
> Sent: Tuesday, November 16, 2010 11:09 AM
> To: Aguirre, Sergio
> Cc: LKML; Huang Ying; Martin Schwidefsky; Ingo Molnar; Kyle McMartin
> Subject: RE: [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure
> 
> On Tue, 2010-11-16 at 10:57 -0600, Aguirre, Sergio wrote:
> 
> > > > @@ -145,7 +145,10 @@ void irq_work_run(void)
> > > >  		 * Clear the BUSY bit and return to the free state if
> > > >  		 * no-one else claimed it meanwhile.
> > > >  		 */
> > > > -		cmpxchg(&entry->next, next_flags(NULL, IRQ_WORK_BUSY),
> NULL);
> > > > +		xchgres = cmpxchg(&entry->next,
> > > > +				  next_flags(NULL, IRQ_WORK_BUSY),
> > > > +				  NULL);
> > > > +		BUG_ON(unlikely(xchgres != next_flags(NULL,
> IRQ_WORK_BUSY)));
> > >
> > > simply adding (void) in front would be much easier.
> >
> > But isn't that still leaving the remote possibility of a hidden cmpxchg
> > Failure open?
> 
> No, we don't care if it fails, read the comment. All we want to know is
> that if it still matched, we flipped the bit.

I understand. Will add just a (void) typecast, and resend then.

Regards,
Sergio

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-11-16 17:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-16 16:32 [RFC][PATCH] irq_work: Don't ignore possible cmpxchg failure Sergio Aguirre
2010-11-16 16:45 ` Peter Zijlstra
2010-11-16 16:57   ` Aguirre, Sergio
2010-11-16 17:08     ` Peter Zijlstra
2010-11-16 17:11       ` Aguirre, Sergio

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).