[linux-pm] non-racy examples, please (was Re: [PATCH 01/13] PM: Add wake lock api. )

[Uli Luckas]

On Sunday 08 February 2009, Pavel Machek wrote:
> Hi!
>
> Ok, I think that this wakelock stuff is in "can't be used properly"
> area on Rusty's scale of nasty interfaces.
>
> So... do I understand this correctly that if I want to run "make zImage"
> on Android, I'll need to modify make, gcc, .... to keep system awake?
>
> 	(How to do that? Give all the userland processes access to
> 	/sys/wakelocks ?)
>
> BTW what does android do when battery goes critically low? I believe
> you want to suspend, ignoring wakelocks, at that point.
>
> And now, you have some X-like system.
>
> 	/* We were idle for too long */
> 	blank_screen();
>
> 	unlock_all_wakelocks();		/* We want machine to sleep */
>
> 	wait_for_key();
> 	/* (here) */
> 	lock_wakelocks("processing_key_wakelock");
>
> ...is that approximately correct? There's race there, right? If (here)
> processing takes too long, or whatever, kernel will sleep the machine
> before it even displays "do you want to unlock the screen" dialog,
> right?
>
> Can you solve that in a way that works, always?
> 									Pavel

Pavel, actually Arve's whole wakelock concept is about avoiding races. Please 
go back to where he explains the keyboard example.
The idea is that the keyboard driver grabs a wake lock in it's interrupt and 
releases the lock only, after the resulting key events are read from the 
device's queue. 
This way, userspace can safely select on the key events, then grab it's own 
lock. At that time, the two locks overlap! After that, userspace reads the 
key events. This causes the keyboard driver to relaease it's wake lock. 
Userspace still holds it's wake lock thogh.
You see? The locks together with the key events have been passed to userspace 
safey. Absolutely race free. 

So your example becomes:
	/* We were idle for too long */
 	blank_screen();
	unlock_my_wakelocks();   /* We want machine to sleep if no one
                                                       else holds a wakelock*/
	wait_for_key();                 /* Select on key events */
	/* (here) */
	lock_wakelocks("processing_key_wakelock");
 	read_key_events();

We should take a look at those cases, though, where a timed wacke locks are 
needed. That is definitely unsafe.

Uli