[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140804210222.GU3935@laptop>
Date: Mon, 4 Aug 2014 23:02:22 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Oleg Nesterov <oleg@...hat.com>
Cc: mingo@...nel.org, torvalds@...ux-foundation.org,
tglx@...utronix.de, ilya.dryomov@...tank.com,
umgwanakikbuti@...il.com, linux-kernel@...r.kernel.org,
Eric Paris <eparis@...isplace.org>,
John McCutchan <john@...nmccutchan.com>,
Robert Love <rlove@...ve.org>
Subject: Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists