lkml.org 
[lkml]   [2022]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v9 12/27] rust: add `kernel` crate
    On Mon, Sep 19, 2022 at 9:09 AM Linus Torvalds
    <torvalds@linux-foundation.org> wrote:
    >
    > The whole "really know what context this code is running within" is
    > really important. You may want to write very explicit comments about
    > it.

    Side note: a corollary of this is that people should avoid "dynamic
    context" things like the plague, because it makes for such pain when
    the context isn't statically obvious.

    So things like conditional locking should generally be avoided as much
    as humanly possible. Either you take the lock or you don't - don't
    write code where the lock context depends on some argument value or
    flag, for example.

    Code like this is fine:

    if (some_condition) {
    spin_lock(&mylock);
    xyz();
    spin_unlock(&mylock);
    }

    because 'xyz()' is always run in the same context. But avoid patterns like

    if (some_condition)
    spin_lock(&mylock);
    xyz();
    if (same_condition)
    spin_unlock(&mylock);

    where now 'xyz()' sometimes does something with the lock held, and
    sometimes not. That way lies insanity.

    Now, obviously, the context for helper functions (like the Rust kernel
    crate is, pretty much by definition) obviously depends on the context
    of the callers of said helpers, so in that sense the above kind of
    "sometimes in locked context, sometimes not" will always be the case.

    So those kinds of helper functions will generally need to be either
    insensitive to context and usable in all contexts (best), or
    documented - and verify with debug code like 'might_sleep()' - that
    they only run in certain contexts.

    And then in the worst case there's a gfp_flag that says "you can only
    do these kinds of allocations" or whatever, but even then you should
    strive to never have other dynamic behavior (ie please try to avoid
    behavior like having a "already locked" argument and then taking a
    lock depending on that).

    Because if you follow those rules, at least you can statically see the
    context from a call chain (so, for example, the stack trace of an oops
    will make the context unambiguous, because there's hopefully no lock
    or interrupt disabling or similar that has some dynamic behavior like
    in that second example of "xyz()".

    Do we have places in the kernel that do conditional locking? Yes we
    do. Examples like that second case do exist. It's bad. Sometimes you
    can't avoid it. But you can always *strive* to avoid it, and
    minimizing those kinds of "context depends on other things"
    situations.

    And we should strive very hard to make those kinds of contexts very
    clear and explicit and not dynamic exactly because it's so important
    in the kernel, and it has subtle implications wrt other locking, and
    memory allocations.

    Linus

    \
     
     \ /
      Last update: 2022-09-19 19:22    [W:2.322 / U:0.916 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site