linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Esben Haabendal <esbenhaabendal@gmail.com>
Cc: joachim.eastwood@jotron.com,
	Peter Zijlstra <peterz@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Esben Haabendal <eha@doredevelopment.dk>,
	linuxppc-dev@ozlabs.org, Marc Zyngier <maz@misterjones.org>,
	Ingo Molnar <mingo@elte.hu>, David Miller <davem@davemloft.net>
Subject: Re: [RFC][PATCH] irq: support IRQ_NESTED_THREAD with non-threaded interrupt handlers
Date: Tue, 8 Jun 2010 01:18:09 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LFD.2.00.1006080015570.2933@localhost.localdomain> (raw)
In-Reply-To: <AANLkTilPhof5cnhBCXSzKJSnWl6Vuu2fRts2pBBBNl6K@mail.gmail.com>

On Mon, 7 Jun 2010, Esben Haabendal wrote:
> On Mon, Jun 7, 2010 at 5:06 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > Maybe you understand now, why I was pretty sure upfront, that your
> > approach was wrong even without knowing all the gory details ? :)
> 
> I understand.  There is a better solution, which is to use threaded
> interrupts where needed.
> 
> But I must confess that I am disappointed that you still fail to see
> how the pca953x
> patch actually eliminates the need for serialization. But I don't
> think there is much
> point in going on about that.

I return the favour of being disappointed that you are still failing
to accept that there is a fundamental difference between "it works in
a particular case" and semantical correctness under all
circumstances.

There is no place for "it works in a particular case" in a kernel
which runs on a gazillion of different devices unless you want to
create a complete unmaintainable mess. The only way to avoid this is
to be strict about semantics even if it seems unsensible for a
particular case.

Also I explained it to you in great length, that your patch violates
the semantical guarantees of existing functions, but you ignore that
completely.

It's Open Source, go wild and change it for your particular case - but
don't expect that I accept patches to the generic irq code which will
cause _me_ predictable trouble as I have to deal with the
fallout. Neither expect, that I ack patches to users of that code
which are semantically wrong.

Just for the record, the pca953x driver is broken vs. the local state
protection anyway as the GPIO functions are not serialized against the
interrupt controller functions. There is no magic which prevents that,
neither on your system nor on any other. The GPIO function should take
buslock when modifying local state and that's one of the reasons why
buslock it is there and cannot go away.

I'm anticipating that you are going to tell me that it does not matter
on your particular powerpc combined with your usage pattern (i.e no
GPIO users). And I tell you right away, that I'm not interested in
this answer for well explained reasons.

> Oh, I still think that the disable_irq_nosync documentaiton is misleading.
> Functions that are allowed in a particular context should not call functions
> that are not allowed to be called in that context. But now I know :-)

As I said before: I agree and I'm happy to accept a patch to fix that
documentation problem.

Thanks,

	tglx

  reply	other threads:[~2010-06-07 23:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1275686352.2970.2.camel@eha.doredevelopment.dk>
     [not found] ` <alpine.LFD.2.00.1006042343150.2933@localhost.localdomain>
     [not found]   ` <AANLkTilb7---Qg1oLwzanKP1RGe04fkAXDtZzz0w9MGz@mail.gmail.com>
     [not found]     ` <20100605151031.2d562268@hina.wild-wind.fr.eu.org>
     [not found]       ` <alpine.LFD.2.00.1006051732270.2933@localhost.localdomain>
     [not found]         ` <AANLkTimlSBEhl58CNuScKjxbqdTwTYgqo7vrHTcd1x9G@mail.gmail.com>
     [not found]           ` <alpine.LFD.2.00.1006051915290.2933@localhost.localdomain>
     [not found]             ` <AANLkTimcB9vRqnNYyBh6hjHXZHcmFvw72MZUEIGfeexq@mail.gmail.com>
     [not found]               ` <alpine.LFD.2.00.1006052155100.2933@localhost.localdomain>
     [not found]                 ` <AANLkTik2so7osQJFq6NRaIFK2GVLURVozOqSsejd1K0F@mail.gmail.com>
     [not found]                   ` <alpine.LFD.2.00.1006062228120.2933@localhost.localdomain>
2010-06-07 12:34                     ` [RFC][PATCH] irq: support IRQ_NESTED_THREAD with non-threaded interrupt handlers Esben Haabendal
2010-06-07 15:06                       ` Thomas Gleixner
2010-06-07 21:28                         ` Esben Haabendal
2010-06-07 23:18                           ` Thomas Gleixner [this message]
2010-06-08 14:15                             ` Esben Haabendal
2010-06-08  6:58                           ` Thomas Gleixner
2010-06-08  7:23                             ` Esben Haabendal

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=alpine.LFD.2.00.1006080015570.2933@localhost.localdomain \
    --to=tglx@linutronix.de \
    --cc=davem@davemloft.net \
    --cc=eha@doredevelopment.dk \
    --cc=esbenhaabendal@gmail.com \
    --cc=joachim.eastwood@jotron.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=maz@misterjones.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.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 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).