Re: [kerneloops] regression in 2.6.27 wrt "lock_page" and the "hwclock" program

[Arjan van de Ven <arjan@infradead.org>]

On Sun, 5 Oct 2008 10:27:42 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> > static unsigned long   
> > atomic(const char *name, unsigned long (*op)(unsigned long),
> >        unsigned long arg)
> > {
> >   unsigned long v;
> >   __asm__ volatile ("cli");
> >   v = (*op)(arg);
> >   __asm__ volatile ("sti");
> >   return v;
> > } 
> > 
> > looks like it (but only on 32 bit x86, not on 64 bit x86)
> 
> I suspect this is new in hwclock?  We do a might_sleep() in
> lock_page() in 2.6.25 and in 2.6.26.

this quote was from the F9 hwclock.. which shipped with 2.6.25.
Hum.

> > > Really, it's a bit stupid doing _any_ system calls (and a
> > > pagefault is a syscall in disguise) with interrupts disabled.
> > > The kernel makes no guarantees that we'll honour it.  We could
> > > just enable interrupts on pagefault entry - that'll teach 'em.
> > 
> > or save - enable - <run handlers> - restore sequence
> 
> hwclock is buggy either way - 

not arguing with that ;-)
All code doing cli/sti in userland is buggy period. No excuses possible.

> it's trying to disable interrupts but
> it's calling into the kernel, which will reenable interrupts, thus
> losing any protection which hwclock was trying to attain.
> 
> Plus there's this little thing called "smp".  I bet it doesn't disable
> interrupts on all CPUs.

I get the impression from the code that it really wants a "don't
schedule me out" rather than "this is a lock".
it can do better.
On Alpha it implements a seq-lock kind of thing instead.
(and on x86-64 .. it implements NOTHING)



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org