lkml.org 
[lkml]   [2017]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] firmware: wake all waiters
    On Fri, Jun 23, 2017 at 4:37 PM, Jakub Kicinski
    <jakub.kicinski@netronome.com> wrote:
    > - swake_up(&fw_st->wq);
    > + swake_up_all(&fw_st->wq);

    Why is that code using the braindamaed "swait" model in the first place?

    That's the real problem here - swait() is a very specialized
    interface, and it does not make sense to use it here.

    Among all the simplifications it has is exactly the fact that it wakes
    up only one thing, because it is *so* specialized.

    But the *only* reason for swait is extreme memory issues and some very
    special realtime issues, where it saves a couple of bytes in
    structures that need close packing, and doesn't even use normal
    spinlocks, so it saves a couple of cycles at wakeup/sleep because it
    doesn't do a good job in general.

    The "avoid normal spinlocks" is because it is meant for code that is
    *so* special that it needs the magical low-level raw spinlocks.

    I really have *no* idea why the firmware code uses that idiotic
    special wait-queue. It has no reason to do so, except this comment
    from the commit that added it:

    "We use also swait instead of wait because don't need all the additional
    features wait provides."

    which is bogus, since it clearly just got the waiting wrong exactly
    *because* swait is pretty damn bad and specialized.

    I think the two valid users are RCU (which needed it for RT), and kvm
    (which also needed it for similar issues - it needs to be
    non-preemptible).

    I don't see any similar reason for the firmware loading, and all it
    did was use an odd interface that resulted in this bug.

    Why is the firmware code being so damn odd on purpose?

    Linus

    \
     
     \ /
      Last update: 2017-06-26 23:45    [W:3.011 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site