linux-kernel - Re: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 29 Nov 2007 13:36:05 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Remy Bohmer <linux@...mer.net>
Cc:	Steven Rostedt <rostedt@...dmis.org>,
	Daniel Walker <dwalker@...sta.com>,
	RT <linux-rt-users@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	ARM Linux Mailing List 
	<linux-arm-kernel@...ts.arm.linux.org.uk>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever

On Thu, Nov 29, 2007 at 12:27:30PM +0100, Remy Bohmer wrote:
> Hello Russell,
> 
> > If 'no' then it's the right handler and the mask/unmask methods associated
> > with the interrupt will be no-ops.
> 
> I completely understand what you keep on saying, but that would imply
> that the following piece of code in chip.c is completely bogus anyway!
> (snip from mainline 2.6.23)

It _is_ bogus.

> handle_simple_irq(unsigned int irq, struct irq_desc *desc)
> ...
> 	if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
> 		if (desc->chip->mask)
> 			desc->chip->mask(irq);
> ...
> }
> 
> Why trying to mask the interrupt if that is illegal use of the interrupt
> type?

No idea, but that code is wrong.  Take a look at the original code
which patch 3692/1 removed.  You'll notice that it never touches
the mask/unmask handlers.

> This piece of code assumes that there CAN be a handler for masking
> interrupts for the simple_irq type. This is in contradiction what you
> are trying to explain.

Shrug.  I'm explaining how _I_ designed it, and the purpose of it being
there.

If people insist on adding the mask/unmask crap to it, the function
might as well be deleted and be an alias for handle_level_IRQ.  Because
that's _precisely_ what you lot are turning it into.

Ah, and looking at the changes to the file, the addition of the mask
and unmask was done by someone who didn't understand what this was
trying to do.  So that change should be backed out.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/