Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

Nick Piggin
2006-07-07T15:08:42Z

Haavard Skinnemoen wrote:
> Hi everyone,
>
> I've put up an updated set of patches for AVR32 support at
> http://avr32linux.org/twiki/bin/view/Main/LinuxPatches
>
> The most interesting patch probably is
> http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-2.patch
>
> which, at 544K, is too large to attach here. Please let me know if you
> want me to do it anyway.
>
> Anyone want to have a look at this? I understand that a full review is
> a huge job, but I'd appreciate a pointer or two in the general
> direction that I need to take this in order to get it acceptable for
> mainline.
>

Hi,

+void cpu_idle(void)
+{
+ /* endless idle loop with no priority at all */
+ while (1) {
+ /* TODO: Enter sleep mode */
+ if (need_resched())
+ schedule();
+ }
+}

AFAIKS, this is buggy.

need_resched() translates to a test_bit, which doesn't have any barriers,
so it could be optimised away completely. And if you're intending to use
preempt, you need to have preemption disabled in the idle loop.

Documentation/sched-arch.txt attempts to explain, and something like
arm26's cpu_idle() is a nice, simple example to follow.

Actually, I'm wrong about the test_bit. It casts to volatile there, which
is probably why you don't end up with infinite loops. Still, it would be
nicer to have an explicit barrier (eg. cpu_relax()).

Why do we cast to volatile in places like this? Linus? I don't see why
test_bit() should be any more "special" than the & operator. What's more,
some architectures do cast and others don't, which is just insane.

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com