Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Aug 04, 2014 at 09:23:58PM +0200, Oleg Nesterov wrote:
> On 08/04, Peter Zijlstra wrote:
> >
> >  	while (1) {
> > -		prepare_to_wait(&group->notification_waitq, &wait, TASK_INTERRUPTIBLE);
> > -
> >  		mutex_lock(&group->notification_mutex);
> 
> So yes, even these 2 lines look obviously buggy. Even if
> fsnotify_add_notify_event()->wake_up(&group->notification_waitq) uses
> TASK_NORMAL, so at least this can't miss an event.

There's another problem, mutex_lock() actively assumes ->state ==
TASK_RUNNING and if its not can go to sleep, possibly without ever being
woken again (because nobody knows its sleeping).

We should probably fix that too, but then its not too weird an
assumption for a blocking primitive.

> It is too later for me, but I am wondering if we can do another thing.
> Something like
> 
> 		int state;
> 
> 		prepare_to_wait(wait, TASK_INTERRUPTIBLE);
> 
> 		PUSH(&wait, state);
> 		mutex_lock();
> 		mutex_unlock();
> 		POP(&wait, state);
> 
> and, ignoring all races, lack of barriers, etc
> 
> 	#define PUSH(w, s)	s = current->state; current->state = RUNNING;
> 
> 	#define POP(w, s)	current->state = WOKEN(w) ? RUNNING : s;
> 
> Probably not... just curious.

Sure we can do a state stack, but I'm not immediately seeing the benefit
of doing so. Also I don't think we want to encourage people to do things
like this.

-rt does something like that for its spinlock->rt_mutex conversion.

In fact, you only need to push/pop around mutex_lock(), unlock will
never change state.