lkml.org 
[lkml]   [2015]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [tip:locking/core] sched/wait: Fix signal handling in bit wait helpers
On Thu, Dec 10, 2015 at 08:30:01AM +1100, NeilBrown wrote:
> On Wed, Dec 09 2015, Peter Zijlstra wrote:
>
> > On Wed, Dec 09, 2015 at 12:06:33PM +1100, NeilBrown wrote:
> >> On Tue, Dec 08 2015, Peter Zijlstra wrote:
> >>
> >> >>
> >> >
> >> > *sigh*, so that patch was broken.. the below might fix it, but please
> >> > someone look at it, I seem to have a less than stellar track record
> >> > here...
> >>
> >> This new change seems to be more intrusive than should be needed.
> >> Can't we just do:
> >>
> >>
> >> __sched int bit_wait(struct wait_bit_key *word)
> >> {
> >> + long state = current->state;
> >
> > No, current->state can already be changed by this time.
>
> Does that matter?
> It can only have changed to TASK_RUNNING - right?
> In that case signal_pending_state() will return 0 and the bit_wait() acts
> as though the thread was woken up normally (which it was) rather than by
> a signal (which maybe it was too, but maybe that happened just a tiny
> bit later).
>
> As long as signal delivery doesn't change ->state, we should be safe.
> We should even be safe testing ->state *after* the call the schedule().

Blergh, all I've managed to far is to confuse myself further. Even
something like the original (+- the EINTR) should work when we consider
the looping, even when mixed with an occasional spurious wakeup.


int bit_wait()
{
if (signal_pending_state(current->state, current))
return -EINTR;
schedule();
}


This can go wrong against raising a signal thusly:

prepare_to_wait()
1: if (signal_pending_state(current->state, current))
// false, nothing pending
schedule();
set_tsk_thread_flag(t, TIF_SIGPENDING);

<spurious wakeup>

prepare_to_wait()
wake_up_state(t, ...);
2: if (signal_pending_state(current->state, current))
// false, TASK_RUNNING

schedule(); // doesn't block because pending

prepare_to_wait()
3: if (signal_pending_state(current->state, current))
// true, pending




\
 
 \ /
  Last update: 2015-12-10 14:41    [W:0.173 / U:2.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site