lkml.org 
[lkml]   [2021]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 01/13] futex2: Implement wait and wake functions
    From
    Date
    Hi Peter,

    Às 06:02 de 16/02/21, Peter Zijlstra escreveu:
    > On Mon, Feb 15, 2021 at 12:23:52PM -0300, André Almeida wrote:
    >> +static int __futex_wait(struct futexv_head *futexv, unsigned int nr_futexes,
    >> + struct hrtimer_sleeper *timeout)
    >> +{
    >> + int ret;
    >> +
    >> + while (1) {
    >> + int awakened = -1;
    >> +
    >
    > Might be easier to understand if the set_current_state() is here,
    > instead of squirreled away in futex_enqueue().
    >

    I placed set_current_state() inside futex_enqueue() because we need to
    set RUNNING and then INTERRUPTIBLE again for the retry path.

    >> + ret = futex_enqueue(futexv, nr_futexes, &awakened);
    >> +
    >> + if (ret) {
    >> + if (awakened >= 0)
    >> + return awakened;
    >> + return ret;
    >> + }
    >> +
    >> + /* Before sleeping, check if someone was woken */
    >> + if (!futexv->hint && (!timeout || timeout->task))
    >> + freezable_schedule();
    >> +
    >> + __set_current_state(TASK_RUNNING);
    >
    > This is typically after the loop.
    >

    Sorry, which loop?

    >> +
    >> + /*
    >> + * One of those things triggered this wake:
    >> + *
    >> + * * We have been removed from the bucket. futex_wake() woke
    >> + * us. We just need to dequeue and return 0 to userspace.
    >> + *
    >> + * However, if no futex was dequeued by a futex_wake():
    >> + *
    >> + * * If the there's a timeout and it has expired,
    >> + * return -ETIMEDOUT.
    >> + *
    >> + * * If there is a signal pending, something wants to kill our
    >> + * thread, return -ERESTARTSYS.
    >> + *
    >> + * * If there's no signal pending, it was a spurious wake
    >> + * (scheduler gave us a change to do some work, even if we
    >
    > chance?

    Indeed, fixed.

    >
    >> + * don't want to). We need to remove ourselves from the
    >> + * bucket and add again, to prevent losing wakeups in the
    >> + * meantime.
    >> + */
    >
    > Anyway, doing a dequeue and enqueue for spurious wakes is a bit of an
    > anti-pattern that can lead to starvation. I've not actually looked at
    > much detail yet as this is my first read-through, but did figure I'd
    > mention it.
    >

    So we could just leave everything enqueued for spurious wake? I was
    expecting that we would need to do all the work again (including
    rechecking *uaddr == val) to see if we didn't miss a futex_wake()
    between the kernel thread waking (spuriously) and going to sleep again.

    >> +
    >> + ret = futex_dequeue_multiple(futexv, nr_futexes);
    >> +
    >> + /* Normal wake */
    >> + if (ret >= 0)
    >> + return ret;
    >> +
    >> + if (timeout && !timeout->task)
    >> + return -ETIMEDOUT;
    >> +
    >> + if (signal_pending(current))
    >> + return -ERESTARTSYS;
    >> +
    >> + /* Spurious wake, do everything again */
    >> + }
    >> +}

    Thanks,
    André

    \
     
     \ /
      Last update: 2021-02-18 21:13    [W:4.674 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site