2018-03-21 18:59:38

by Sinan Kaya

[permalink] [raw]
Subject: [PATCH REPOST v4 5/7] ixgbevf: keep writel() closer to wmb()

Remove ixgbevf_write_tail() in favor of moving writel() close to
wmb().

Signed-off-by: Sinan Kaya <[email protected]>
Reviewed-by: Alexander Duyck <[email protected]>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 5 -----
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++--
2 files changed, 2 insertions(+), 7 deletions(-)

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:
--
2.7.4



2018-03-21 21:48:56

by Jeff Kirsher

[permalink] [raw]
Subject: Re: [PATCH REPOST v4 5/7] ixgbevf: keep writel() closer to wmb()

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 <[email protected]>
> Reviewed-by: Alexander Duyck <[email protected]>
> ---
> 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.


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part

2018-03-21 21:52:47

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH REPOST v4 5/7] ixgbevf: keep writel() closer to wmb()

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 <[email protected]>
>> Reviewed-by: Alexander Duyck <[email protected]>
>> ---
>> 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.

2018-03-21 21:54:39

by Alexander Duyck

[permalink] [raw]
Subject: Re: [Intel-wired-lan] [PATCH REPOST v4 5/7] ixgbevf: keep writel() closer to wmb()

On Wed, Mar 21, 2018 at 2:51 PM, <[email protected]> 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 <[email protected]>
>>> Reviewed-by: Alexander Duyck <[email protected]>
>>> ---
>>> 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

2018-03-21 21:57:12

by David Miller

[permalink] [raw]
Subject: Re: [PATCH REPOST v4 5/7] ixgbevf: keep writel() closer to wmb()

From: Jeff Kirsher <[email protected]>
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 <[email protected]>
>> Reviewed-by: Alexander Duyck <[email protected]>
>> ---
>> 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.

2018-03-21 22:15:56

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH REPOST v4 5/7] ixgbevf: keep writel() closer to wmb()

On 2018-03-21 17:54, David Miller wrote:
> From: Jeff Kirsher <[email protected]>
> 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 <[email protected]>
>>> Reviewed-by: Alexander Duyck <[email protected]>
>>> ---
>>> 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.