[PATCH 7/7] ixgbevf: eliminate duplicate barriers on weakly-ordered archs

On 3/15/2018 10:32 AM, Alexander Duyck wrote:

We tend to do something like:
  update tx_buffer_info
  update tx_desc
  wmb()
  point first tx_buffer_info next_to_watch value at last tx_desc
  update next_to_use
  notify device via writel

We do it this way because we have to synchronize between the Tx
cleanup path and the hardware so we basically lump the two barriers
together. instead of invoking both a smp_wmb and a wmb. Now that I
look at the pseudocode though I wonder if we shouldn't move the
next_to_use update before the wmb, but that might be material for
another patch. Anyway, in the Tx cleanup path we should have an
smp_rmb() after we read the next_to_watch values so that we avoid
reading any of the other fields in the buffer_info if either the field
is NULL or the descriptor pointed to has not been written back.

How do you feel about keeping wmb() very close to writel_relaxed() like this?

   update tx_buffer_info
   update tx_desc
   point first tx_buffer_info next_to_watch value at last tx_desc
   update next_to_use
   wmb()
   notify device via writel_relaxed()

I'm afraid that if the order of wmb() and writel() is not very
obvious or hidden in multiple functions, somebody can introduce a very nasty
bug in the future.

We also have to think about code maintenance.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.