All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@in.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Bernhard Walle <bwalle@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Fix crash with irqpoll due to the IRQF_IRQPOLL flag
Date: Thu, 24 May 2007 10:58:49 +0530	[thread overview]
Message-ID: <20070524052849.GA26967@in.ibm.com> (raw)
In-Reply-To: <alpine.LFD.0.98.0705230836370.3890@woody.linux-foundation.org>

On Wed, May 23, 2007 at 09:01:04AM -0700, Linus Torvalds wrote:
> 
> 
> On Tue, 22 May 2007, Bernhard Walle wrote:
> >
> > o System crashes if booted with irqpoll command line option.
> > 
> > o Problem happens because Inside note_interrupt() we are accessing
> >   desc->action->flag without taking the desc->lock. While accessing it
> >   somebody goes ahead and unregisters the irq handler hence desc->action
> >   is NULL. By the time note_interrupt() checks it, it crashes.
> 
> I absolutely _detest_ patches that make already complex and unreadable 
> code even more so. Especially conditionals. Please don't do that.
> 
> If you need a variable for a conditional, make it be an implicit one from 
> an inline function, and aim for making it readable.
> 
> So how about instead writing it out as a nice self-explanatory inline 
> function? I can almost guarantee that this generates no worse code, and it 
> also makes it easy to explain things like "we don't bother with the lock, 
> because we don't care enough".
> 
> Untested, but I think the point of the patch is obvious. Anybody want to 
> test it, send it back to me, and fix the bug while making the code more 
> readable?
> 

Hi Linus,

I tested it. It works fine. And yes, this patch is more readable.

Thanks
Vivek

      reply	other threads:[~2007-05-24  5:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-26  9:36 2.6.21-rc7-mm2 "irqpoll" seems to be broken Vivek Goyal
2007-04-26 15:24 ` Andrew Morton
2007-04-26 15:43   ` Bernhard Walle
2007-04-30  8:48   ` Vivek Goyal
2007-05-01 19:20     ` Bernhard Walle
     [not found]     ` <20070502221932.GA488@suse.de>
2007-05-08 17:18       ` Vivek Goyal
2007-05-14 14:05         ` Bernhard Walle
2007-05-17 13:05           ` Vivek Goyal
2007-05-17 21:56             ` Bernhard Walle
2007-05-22  8:37           ` [PATCH] Fix crash with irqpoll due to the IRQF_IRQPOLL flag Bernhard Walle
2007-05-23 16:01             ` Linus Torvalds
2007-05-24  5:28               ` Vivek Goyal [this message]

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=20070524052849.GA26967@in.ibm.com \
    --to=vgoyal@in.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=bwalle@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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 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.