lkml.org 
[lkml]   [2018]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] IB/core: Make ib_mad_client_id atomic
    From
    Date

    +Jack

    > On 20 Apr 2018, at 17:34, Jason Gunthorpe <jgg@ziepe.ca> wrote:
    >
    > On Thu, Apr 19, 2018 at 11:55:55PM -0400, Doug Ledford wrote:
    >> On Wed, 2018-04-18 at 16:24 +0200, Håkon Bugge wrote:
    >>> Two kernel threads may get the same value for agent.hi_tid, if the
    >>> agents are registered for different ports. As of now, this works, as
    >>> the agent list is per port.
    >>>
    >>> It is however confusing and not future robust. Hence, making it
    >>> atomic.
    >>>
    >>
    >> People sometimes underestimate the performance penalty of atomic ops.
    >> Every atomic op is the equivalent of a spin_lock/spin_unlock pair.

    Well, may be this holds true if the mutex and the variable is located in the same cacheline.

    >> This
    >> is why two atomics are worse than taking a spin_lock, doing what you
    >> have to do, and releasing the spin_lock. Is this really what you want
    >> for a "confusing, let's make it robust" issue?
    >
    > But it is on the ib_register_mad_agent() path which is not a
    > performance path..
    >
    > This actually looks like a genuine bug, why is it described only as
    > 'confusing'? ib_register_mad_agent is callable from userspace, so at
    > least two userspace agents can race and get the same TID’s.

    My understanding is that every lookup is using the {port, TID} tuple. As such, it is not a bug, but, very confusing.

    > TIDs need to be globally unique on the entire machine.

    If you are correct Jason, let me reword the commit message.


    Thxs, Håkon

    \
     
     \ /
      Last update: 2018-04-23 16:21    [W:2.939 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site