* [PATCH] net: hisilicon: Fix ping latency when deal with high throughput
@ 2019-10-26 8:49 Jiangfeng Xiao
2019-10-26 15:44 ` Joe Perches
2019-10-26 18:22 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Jiangfeng Xiao @ 2019-10-26 8:49 UTC (permalink / raw)
To: davem, yisen.zhuang, salil.mehta, xiaojiangfeng
Cc: netdev, linux-kernel, leeyou.li, zhanghan23, nixiaoming,
zhangqiang.cn, dingjingcheng
This is due to error in over budget processing.
When dealing with high throughput, the used buffers
that exceeds the budget is not cleaned up. In addition,
it takes a lot of cycles to clean up the used buffer,
and then the buffer where the valid data is located can take effect.
Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
---
drivers/net/ethernet/hisilicon/hip04_eth.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index ad6d912..78f338a 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -575,7 +575,7 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
struct net_device *ndev = priv->ndev;
struct net_device_stats *stats = &ndev->stats;
- unsigned int cnt = hip04_recv_cnt(priv);
+ static unsigned int cnt_remaining;
struct rx_desc *desc;
struct sk_buff *skb;
unsigned char *buf;
@@ -588,8 +588,8 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
/* clean up tx descriptors */
tx_remaining = hip04_tx_reclaim(ndev, false);
-
- while (cnt && !last) {
+ cnt_remaining += hip04_recv_cnt(priv);
+ while (cnt_remaining && !last) {
buf = priv->rx_buf[priv->rx_head];
skb = build_skb(buf, priv->rx_buf_size);
if (unlikely(!skb)) {
@@ -635,11 +635,13 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
hip04_set_recv_desc(priv, phys);
priv->rx_head = RX_NEXT(priv->rx_head);
- if (rx >= budget)
+ if (rx >= budget) {
+ --cnt_remaining;
goto done;
+ }
- if (--cnt == 0)
- cnt = hip04_recv_cnt(priv);
+ if (--cnt_remaining == 0)
+ cnt_remaining += hip04_recv_cnt(priv);
}
if (!(priv->reg_inten & RCV_INT)) {
--
1.8.5.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] net: hisilicon: Fix ping latency when deal with high throughput
2019-10-26 8:49 [PATCH] net: hisilicon: Fix ping latency when deal with high throughput Jiangfeng Xiao
@ 2019-10-26 15:44 ` Joe Perches
2019-10-28 1:24 ` Jiangfeng Xiao
2019-10-26 18:22 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Joe Perches @ 2019-10-26 15:44 UTC (permalink / raw)
To: Jiangfeng Xiao, davem, yisen.zhuang, salil.mehta
Cc: netdev, linux-kernel, leeyou.li, zhanghan23, nixiaoming,
zhangqiang.cn, dingjingcheng
On Sat, 2019-10-26 at 16:49 +0800, Jiangfeng Xiao wrote:
> This is due to error in over budget processing.
> When dealing with high throughput, the used buffers
> that exceeds the budget is not cleaned up. In addition,
> it takes a lot of cycles to clean up the used buffer,
> and then the buffer where the valid data is located can take effect.
[]
> diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
[]
> @@ -575,7 +575,7 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
> struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
> struct net_device *ndev = priv->ndev;
> struct net_device_stats *stats = &ndev->stats;
> - unsigned int cnt = hip04_recv_cnt(priv);
> + static unsigned int cnt_remaining;
static doesn't seem a great idea here as it's for just a single
driver instance. Maybe make this part of struct hip04_priv?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: hisilicon: Fix ping latency when deal with high throughput
2019-10-26 8:49 [PATCH] net: hisilicon: Fix ping latency when deal with high throughput Jiangfeng Xiao
2019-10-26 15:44 ` Joe Perches
@ 2019-10-26 18:22 ` David Miller
2019-10-28 1:27 ` Jiangfeng Xiao
1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2019-10-26 18:22 UTC (permalink / raw)
To: xiaojiangfeng
Cc: yisen.zhuang, salil.mehta, netdev, linux-kernel, leeyou.li,
zhanghan23, nixiaoming, zhangqiang.cn, dingjingcheng
From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
Date: Sat, 26 Oct 2019 16:49:39 +0800
> diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
> index ad6d912..78f338a 100644
> --- a/drivers/net/ethernet/hisilicon/hip04_eth.c
> +++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
> @@ -575,7 +575,7 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
> struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
> struct net_device *ndev = priv->ndev;
> struct net_device_stats *stats = &ndev->stats;
> - unsigned int cnt = hip04_recv_cnt(priv);
> + static unsigned int cnt_remaining;
There is no way a piece of software state should be system wide, this is
a per device instance value.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: hisilicon: Fix ping latency when deal with high throughput
2019-10-26 15:44 ` Joe Perches
@ 2019-10-28 1:24 ` Jiangfeng Xiao
0 siblings, 0 replies; 5+ messages in thread
From: Jiangfeng Xiao @ 2019-10-28 1:24 UTC (permalink / raw)
To: Joe Perches, davem, yisen.zhuang, salil.mehta
Cc: netdev, linux-kernel, leeyou.li, zhanghan23, nixiaoming,
zhangqiang.cn, dingjingcheng
On 2019/10/26 23:44, Joe Perches wrote:
> On Sat, 2019-10-26 at 16:49 +0800, Jiangfeng Xiao wrote:
>> This is due to error in over budget processing.
>> When dealing with high throughput, the used buffers
>> that exceeds the budget is not cleaned up. In addition,
>> it takes a lot of cycles to clean up the used buffer,
>> and then the buffer where the valid data is located can take effect.
> []
>> diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
> []
>> @@ -575,7 +575,7 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
>> struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
>> struct net_device *ndev = priv->ndev;
>> struct net_device_stats *stats = &ndev->stats;
>> - unsigned int cnt = hip04_recv_cnt(priv);
>> + static unsigned int cnt_remaining;
>
> static doesn't seem a great idea here as it's for just a single
> driver instance. Maybe make this part of struct hip04_priv?
>
Thank you for your review. Your suggestion is very good.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: hisilicon: Fix ping latency when deal with high throughput
2019-10-26 18:22 ` David Miller
@ 2019-10-28 1:27 ` Jiangfeng Xiao
0 siblings, 0 replies; 5+ messages in thread
From: Jiangfeng Xiao @ 2019-10-28 1:27 UTC (permalink / raw)
To: David Miller
Cc: yisen.zhuang, salil.mehta, netdev, linux-kernel, leeyou.li,
zhanghan23, nixiaoming, zhangqiang.cn, dingjingcheng
On 2019/10/27 2:22, David Miller wrote:
> From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
> Date: Sat, 26 Oct 2019 16:49:39 +0800
>
>> diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
>> index ad6d912..78f338a 100644
>> --- a/drivers/net/ethernet/hisilicon/hip04_eth.c
>> +++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
>> @@ -575,7 +575,7 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
>> struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
>> struct net_device *ndev = priv->ndev;
>> struct net_device_stats *stats = &ndev->stats;
>> - unsigned int cnt = hip04_recv_cnt(priv);
>> + static unsigned int cnt_remaining;
>
> There is no way a piece of software state should be system wide, this is
> a per device instance value.
>
> .
>
Thank you for your guidance, I will fix it in v2.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-28 1:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-26 8:49 [PATCH] net: hisilicon: Fix ping latency when deal with high throughput Jiangfeng Xiao
2019-10-26 15:44 ` Joe Perches
2019-10-28 1:24 ` Jiangfeng Xiao
2019-10-26 18:22 ` David Miller
2019-10-28 1:27 ` Jiangfeng Xiao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).