[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200711140020.46764.david-b@pacbell.net>
Date: Wed, 14 Nov 2007 00:20:46 -0800
From: David Brownell <david-b@...bell.net>
To: Nick Piggin <nickpiggin@...oo.com.au>
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 Tuesday 13 November 2007, Nick Piggin wrote:
> > > > I'd be happy if, as originally presented, it were possible to just
> > > > pass a raw_spinlock_t to spin_lock_irqsave() and friends.
> > >
> > > that's a spinlock type abstraction of PREEMPT_RT, not of mainline.
>
> Even when you're talking about the -rt tree, I suspect you really
> shouldn't be using raw spinlocks, right?
That's one subject of discussion here. I got the expected
knee-jerk responses (saying just that) -- right, ignore those
and aim for ones that look at the actual issues.
> 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 ...
Today, platform GPIO logic has no preemption points.
There are a few dozen implementations in the tree, not all
using the generic calls (yet!), none radically different.
Certainly when gpio_set_value() is inlined, it works very
much like set_bit()/clear_bit() on hardware with atomic ops.
Not a preemption point
But it often turns into a function call that needs to map
from GPIO number to GPIO bank, and then to a bitslice through
that bank's registers. I know of only one platform which
even has any locking on those code paths(*) ... so again
those calls aren't preemption points.
> 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).
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.
> > 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...
I think you've just presented an argument why that stuff
shouldn't really exist in -rt either... :)
- Dave
(*) Exception, arch/arm/plat-omap/gpio.c uses spinlocks.
Maybe I should say "misuses"; ISTR getting various
lockdep complaints about the same spinlock being used
both from IRQ and non-IRQ contexts. In some common
cases that lock could probably be eliminated, but
if you look at the code you'll know why nobody's much
wanted to clean it up.
-
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/
Powered by blists - more mailing lists