Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Nick Piggin <nickpiggin@yahoo.com.au>]

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