linux-kernel - Re: [patch 2.6.24-rc2 1/3] generic gpio -- gpio

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]

Message-Id: <200711140818.59714.nickpiggin@yahoo.com.au>
Date:	Wed, 14 Nov 2007 08:18:59 +1100
From:	Nick Piggin <nickpiggin@...oo.com.au>
To:	David Brownell <david-b@...bell.net>
Cc:	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel list <linux-kernel@...r.kernel.org>,
	Florian Fainelli <florian.fainelli@...ecomint.eu>,
	Haavard Skinnemoen <hskinnemoen@...el.com>
Subject: Re: [patch 2.6.24-rc2 1/3] generic gpio -- gpio_chip support

On Wednesday 14 November 2007 19:20, David Brownell wrote:
> On Tuesday 13 November 2007, Nick Piggin wrote:

> > I mean, if you have a
> > timing critical operation, then you should ensure you have priorities
> > set correctly so that you simply don't get preempted.
>
> Which is why bitops like <asm-generic/bitops/atomic.h> use
> normal spinlocks.  Oh, wait, no they don't ...

No it isn't. It's nothing to do with that because upstream raw
spinlocks disable preemption as well; the reason for using raw
spinlocks in atomic.h is completely different.

Anyway, this whole line of argument is flawed. Even if the atomic.h
code is crap, that doesn't give any license to introduce more bad
code.

> > By using a raw_spinlock_t, you're saying that you're more important
> > than anyone else (for the period of the critical section) including
> > processes which the user has explicitly set to a higher priority.
>
> Nope.  Just saying that the relevant instructions (three, in the
> hot path I looked at, and not a case where priority inversion
> scenarios should be a concern) shouldn't be forcibly morphed into
> preemption points.  Any more than other bitops were (not).

Don't raw_spinlock_t's quite explictly disallow preemption in the
critical section? Eg. as opposed to spinlock_t, which does not, in 
-rt.

> If a higher priority task needs that CPU, nothing prevents it
> from being immediately descheduled.  Ditto handling a hardirq.
>
> All this does is prevent constant and needless checking for
> "do you want to preempt me now?" "now?" "now?" in "now?" the
> middle "now?" of "now?" i/o "now?" loops.

Actually that's wrong. By disabling preemption, you have to
explicitly check whether you should be preempted when you enable
it again. If you don't disable preemption then you don't have to
check anything -- you'll simply be preempted by asynchronous
events.

> > > Any reason that stuff shouldn't move into mainline?
> >
> > This sort of raw_spinlock_t arms race throughout drivers/ would be
> > a huge reason not to move it into mainline.
>
> This isn't driver code...

Semantics. It's not something like the scheduler or interrupt handler
or something that might have real reasons to use raw locks.

> I think you've just presented an argument why that stuff
> shouldn't really exist in -rt either... :)

Anyway, I'm not going to argue about -rt specific stuff any longer.
Whatever. It shouldn't go upstream like this though.
-
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/