| Message ID | 1521658572-26354-6-git-send-email-okaya@codeaurora.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | netdev: intel: Eliminate duplicate barriers on weakly-ordered archs | expand |
On Wed, 2018-03-21 at 14:56 -0400, Sinan Kaya wrote: > Remove ixgbevf_write_tail() in favor of moving writel() close to > wmb(). > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com> > --- > drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 5 ----- > drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++-- > 2 files changed, 2 insertions(+), 7 deletions(-) This patch fails to compile because there is a call to ixgbevf_write_tail() which you missed cleaning up.
On 2018-03-21 17:48, Jeff Kirsher wrote: > On Wed, 2018-03-21 at 14:56 -0400, Sinan Kaya wrote: >> Remove ixgbevf_write_tail() in favor of moving writel() close to >> wmb(). >> >> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >> Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com> >> --- >> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 5 ----- >> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++-- >> 2 files changed, 2 insertions(+), 7 deletions(-) > > This patch fails to compile because there is a call to > ixgbevf_write_tail() which you missed cleaning up. Hah, I did a compile test but maybe I missed something. I will get v6 of this patch only and leave the rest of the series as it is.
On Wed, Mar 21, 2018 at 2:51 PM, <okaya@codeaurora.org> wrote: > On 2018-03-21 17:48, Jeff Kirsher wrote: >> >> On Wed, 2018-03-21 at 14:56 -0400, Sinan Kaya wrote: >>> >>> Remove ixgbevf_write_tail() in favor of moving writel() close to >>> wmb(). >>> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >>> Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com> >>> --- >>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 5 ----- >>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++-- >>> 2 files changed, 2 insertions(+), 7 deletions(-) >> >> >> This patch fails to compile because there is a call to >> ixgbevf_write_tail() which you missed cleaning up. > > > Hah, I did a compile test but maybe I missed something. I will get v6 of > this patch only and leave the rest of the series as it is. Actually you might want to just pull Jeff's tree and rebase before you submit your patches. I suspect the difference is the ixgbevf XDP code that is present in Jeff's tree and not in Dave's. The alternative is to wait for Jeff to push the ixgbevf code and then once Dave has pulled it you could rebase your patches. Thanks. - Alex
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Date: Wed, 21 Mar 2018 14:48:08 -0700 > On Wed, 2018-03-21 at 14:56 -0400, Sinan Kaya wrote: >> Remove ixgbevf_write_tail() in favor of moving writel() close to >> wmb(). >> >> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >> Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com> >> --- >> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 5 ----- >> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++-- >> 2 files changed, 2 insertions(+), 7 deletions(-) > > This patch fails to compile because there is a call to > ixgbevf_write_tail() which you missed cleaning up. For a change with delicate side effects, it doesn't create much confidence if the code does not even compile. Sinan, please put more care into the changes you are making. Thank you.
On 2018-03-21 17:54, David Miller wrote: > From: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > Date: Wed, 21 Mar 2018 14:48:08 -0700 > >> On Wed, 2018-03-21 at 14:56 -0400, Sinan Kaya wrote: >>> Remove ixgbevf_write_tail() in favor of moving writel() close to >>> wmb(). >>> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >>> Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com> >>> --- >>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 5 ----- >>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++-- >>> 2 files changed, 2 insertions(+), 7 deletions(-) >> >> This patch fails to compile because there is a call to >> ixgbevf_write_tail() which you missed cleaning up. > > For a change with delicate side effects, it doesn't create much > confidence if the code does not even compile. > > Sinan, please put more care into the changes you are making. I think the issue is the tree that code is getting tested has undelivered code as Alex mentioned. I was using linux-next 4.16 rc4 for testing. I will rebase to Jeff's tree. > > Thank you.
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h index f695242..11e893e 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h @@ -244,11 +244,6 @@ static inline u16 ixgbevf_desc_unused(struct ixgbevf_ring *ring) return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 1; } -static inline void ixgbevf_write_tail(struct ixgbevf_ring *ring, u32 value) -{ - writel(value, ring->tail); -} - #define IXGBEVF_RX_DESC(R, i) \ (&(((union ixgbe_adv_rx_desc *)((R)->desc))[i])) #define IXGBEVF_TX_DESC(R, i) \ diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c index 9b3d43d..6bf778a 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c @@ -659,7 +659,7 @@ static void ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring, * such as IA-64). */ wmb(); - ixgbevf_write_tail(rx_ring, i); + writel(i, rx_ring->tail); } } @@ -3644,7 +3644,7 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring, tx_ring->next_to_use = i; /* notify HW of packet */ - ixgbevf_write_tail(tx_ring, i); + writel(i, tx_ring->tail); return; dma_error: