• Jacob Keller's avatar
    fm10k: use a BITMAP for flags to avoid race conditions · 3ee7b3a3
    Jacob Keller authored
    Replace bitwise operators and #defines with a BITMAP and enumeration
    values. This is similar to how we handle the "state" values as well.
    
    This has two distinct advantages over the old method. First, we ensure
    correctness of operations which are currently problematic due to race
    conditions. Suppose that two kernel threads are running, such as the
    watchdog and an ethtool ioctl, and both modify flags. We'll say that the
    watchdog is CPU A, and the ethtool ioctl is CPU B.
    
    CPU A sets FLAG_1, which can be seen as
      CPU A read FLAGS
      CPU A write FLAGS | FLAG_1
    
    CPU B sets FLAG_2, which can be seen as
      CPU B read FLAGS
      CPU A write FLAGS | FLAG_2
    
    However, "|=" and "&=" operators are not actually atomic. So this could
    be ordered like the following:
    
    CPU A read FLAGS -> variable
    CPU B read FLAGS -> variable
    CPU A write FLAGS (variable | FLAG_1)
    CPU B write FLAGS (variable | FLAG_2)
    
    Notice how the 2nd write from CPU B could actually undo the write from
    CPU A because it isn't guaranteed that the |= operation is atomic.
    
    In practice the race windows for most flag writes is incredibly narrow
    so it is not easy to isolate issues. However, the more flags we have,
    the more likely they will cause problems. Additionally, if such
    a problem were to arise, it would be incredibly difficult to track down.
    
    Second, there is an additional advantage beyond code correctness. We can
    now automatically size the BITMAP if more flags were added, so that we
    do not need to remember that flags is u32 and thus if we added too many
    flags we would over-run the variable. This is not a likely occurrence
    for fm10k driver, but this patch can serve as an example for other
    drivers which have many more flags.
    
    This particular change does have a bit of trouble converting some of the
    idioms previously used with the #defines for flags. Specifically, when
    converting FM10K_FLAG_RSS_FIELD_IPV[46]_UDP flags. This whole operation
    was actually quite problematic, because we actually stored flags
    separately. This could more easily show the problem of the above
    re-ordering issue.
    
    This is really difficult to test whether atomics make a difference in
    practical scenarios, but you can ensure that basic functionality remains
    the same. This patch has a lot of code coverage, but most of it is
    relatively simple.
    
    While we are modifying these files, update their copyright year.
    Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
    Tested-by: default avatarKrishneil Singh <krishneil.k.singh@intel.com>
    Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
    3ee7b3a3
fm10k_main.c 54.5 KB