* [PATCH net 0/2] fix napi return value
@ 2015-01-20 13:16 Govindarajulu Varadarajan
2015-01-20 13:16 ` [PATCH net 1/2] enic: fix rx napi poll " Govindarajulu Varadarajan
2015-01-20 13:16 ` [PATCH net 2/2] bnx2x: fix napi poll return value for repoll Govindarajulu Varadarajan
0 siblings, 2 replies; 10+ messages in thread
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
Since d75b1ade567ffab ("net: less interrupt masking in NAPI") we should return
budget for napi repoll.
In enic, we return 0 when busy_poll is happening. Fix this by returning budget.
Looks like bnx2x needs this fix as well. On bnx2x I performed compile
test only. I do not have hardware.
Govindarajulu Varadarajan (2):
enic: fix rx napi poll return value
bnx2x: fix napi poll return value for repoll
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++--
drivers/net/ethernet/cisco/enic/enic_main.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
--
2.2.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 1/2] enic: fix rx napi poll return value
2015-01-20 13:16 [PATCH net 0/2] fix napi return value Govindarajulu Varadarajan
@ 2015-01-20 13:16 ` Govindarajulu Varadarajan
2015-01-25 6:39 ` David Miller
2015-01-20 13:16 ` [PATCH net 2/2] bnx2x: fix napi poll return value for repoll Govindarajulu Varadarajan
1 sibling, 1 reply; 10+ messages in thread
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
is done only when work_done == budget. When we are in busy_poll we return 0 in
napi_poll. We should return budget.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index b29e027..e356afa 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1335,7 +1335,7 @@ static int enic_poll_msix_rq(struct napi_struct *napi, int budget)
int err;
if (!enic_poll_lock_napi(&enic->rq[rq]))
- return work_done;
+ return budget;
/* Service RQ
*/
--
2.2.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
2015-01-20 13:16 [PATCH net 0/2] fix napi return value Govindarajulu Varadarajan
2015-01-20 13:16 ` [PATCH net 1/2] enic: fix rx napi poll " Govindarajulu Varadarajan
@ 2015-01-20 13:16 ` Govindarajulu Varadarajan
2015-01-20 14:51 ` Eric Dumazet
2015-01-25 6:40 ` David Miller
1 sibling, 2 replies; 10+ messages in thread
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
is done only when work_done == budget. When in busy_poll is we return 0 in
napi_poll. We should return budget. Also do not return workdone > budget.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 1d1147c..ebcbe92 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -3175,7 +3175,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
}
#endif
if (!bnx2x_fp_lock_napi(fp))
- return work_done;
+ return budget;
for_each_cos_in_tx_queue(fp, cos)
if (bnx2x_tx_queue_has_work(fp->txdata_ptr[cos]))
@@ -3187,7 +3187,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
/* must not complete if we consumed full budget */
if (work_done >= budget) {
bnx2x_fp_unlock_napi(fp);
- break;
+ return budget;
}
}
--
2.2.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
2015-01-20 13:16 ` [PATCH net 2/2] bnx2x: fix napi poll return value for repoll Govindarajulu Varadarajan
@ 2015-01-20 14:51 ` Eric Dumazet
2015-01-20 14:54 ` Eric Dumazet
2015-01-20 15:24 ` Govindarajulu Varadarajan
2015-01-25 6:40 ` David Miller
1 sibling, 2 replies; 10+ messages in thread
From: Eric Dumazet @ 2015-01-20 14:51 UTC (permalink / raw)
To: Govindarajulu Varadarajan, Willem de Bruijn
Cc: davem, netdev, ariel.elior, ssujith, benve
On Tue, 2015-01-20 at 18:46 +0530, Govindarajulu Varadarajan wrote:
> With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
> is done only when work_done == budget. When in busy_poll is we return 0 in
> napi_poll. We should return budget. Also do not return workdone > budget.
>
I am not sure.
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 1d1147c..ebcbe92 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -3175,7 +3175,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
> }
> #endif
> if (!bnx2x_fp_lock_napi(fp))
> - return work_done;
> + return budget;
For this one I am not sure.
Busy poll is not supposed to drain the whole queue.
>
> for_each_cos_in_tx_queue(fp, cos)
> if (bnx2x_tx_queue_has_work(fp->txdata_ptr[cos]))
> @@ -3187,7 +3187,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
> /* must not complete if we consumed full budget */
> if (work_done >= budget) {
> bnx2x_fp_unlock_napi(fp);
> - break;
> + return budget;
This one looks fine.
> }
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
2015-01-20 14:51 ` Eric Dumazet
@ 2015-01-20 14:54 ` Eric Dumazet
2015-01-20 15:26 ` Govindarajulu Varadarajan
2015-01-20 15:24 ` Govindarajulu Varadarajan
1 sibling, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2015-01-20 14:54 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: Willem de Bruijn, davem, netdev, ariel.elior, ssujith, benve
On Tue, 2015-01-20 at 06:51 -0800, Eric Dumazet wrote:
> >
> > for_each_cos_in_tx_queue(fp, cos)
> > if (bnx2x_tx_queue_has_work(fp->txdata_ptr[cos]))
> > @@ -3187,7 +3187,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
> > /* must not complete if we consumed full budget */
> > if (work_done >= budget) {
> > bnx2x_fp_unlock_napi(fp);
> > - break;
> > + return budget;
>
> This one looks fine.
But its not necessary, as here budget == work_done.
(work_done > budget) would be a bug from bnx2x_rx_int()
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
2015-01-20 14:51 ` Eric Dumazet
2015-01-20 14:54 ` Eric Dumazet
@ 2015-01-20 15:24 ` Govindarajulu Varadarajan
2015-01-20 15:44 ` Eric Dumazet
1 sibling, 1 reply; 10+ messages in thread
From: Govindarajulu Varadarajan @ 2015-01-20 15:24 UTC (permalink / raw)
To: Eric Dumazet
Cc: Govindarajulu Varadarajan, Willem de Bruijn, davem, netdev,
ariel.elior, ssujith, benve
On Tue, 20 Jan 2015, Eric Dumazet wrote:
> On Tue, 2015-01-20 at 18:46 +0530, Govindarajulu Varadarajan wrote:
>> With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
>> is done only when work_done == budget. When in busy_poll is we return 0 in
>> napi_poll. We should return budget. Also do not return workdone > budget.
>>
>
> I am not sure.
>
This is based on f41281d02f8b94e136f78cb1b6a5d78182c222bd &
9dfa9a27b620640322588df399eb8f624b48d877
I do not know about bnx2x, but in enic driver, when busy_poll is enables
rq clean is not happening. This is because, in napi_poll() when
work_done < budget, we do not repoll. At this point, enic has disables rq intr
and has not called napi_complete. Driver assumes that napi will repoll.
Which does not happen.
Lot of drivers I have checked return full budget if they want to repoll.
eg. mlx4_en_poll_rx_cq()
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
2015-01-20 14:54 ` Eric Dumazet
@ 2015-01-20 15:26 ` Govindarajulu Varadarajan
0 siblings, 0 replies; 10+ messages in thread
From: Govindarajulu Varadarajan @ 2015-01-20 15:26 UTC (permalink / raw)
To: Eric Dumazet
Cc: Govindarajulu Varadarajan, Willem de Bruijn, davem, netdev,
ariel.elior, ssujith, benve
On Tue, 20 Jan 2015, Eric Dumazet wrote:
> On Tue, 2015-01-20 at 06:51 -0800, Eric Dumazet wrote:
>
>>>
>>> for_each_cos_in_tx_queue(fp, cos)
>>> if (bnx2x_tx_queue_has_work(fp->txdata_ptr[cos]))
>>> @@ -3187,7 +3187,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
>>> /* must not complete if we consumed full budget */
>>> if (work_done >= budget) {
>>> bnx2x_fp_unlock_napi(fp);
>>> - break;
>>> + return budget;
>>
>> This one looks fine.
>
> But its not necessary, as here budget == work_done.
>
> (work_done > budget) would be a bug from bnx2x_rx_int()
>
Yes, I missed that one. This change here is not needed.
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
2015-01-20 15:24 ` Govindarajulu Varadarajan
@ 2015-01-20 15:44 ` Eric Dumazet
0 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2015-01-20 15:44 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: Willem de Bruijn, davem, netdev, ariel.elior, ssujith, benve
On Tue, 2015-01-20 at 20:54 +0530, Govindarajulu Varadarajan wrote:
> On Tue, 20 Jan 2015, Eric Dumazet wrote:
>
> > On Tue, 2015-01-20 at 18:46 +0530, Govindarajulu Varadarajan wrote:
> >> With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
> >> is done only when work_done == budget. When in busy_poll is we return 0 in
> >> napi_poll. We should return budget. Also do not return workdone > budget.
> >>
> >
> > I am not sure.
> >
>
> This is based on f41281d02f8b94e136f78cb1b6a5d78182c222bd &
> 9dfa9a27b620640322588df399eb8f624b48d877
>
> I do not know about bnx2x, but in enic driver, when busy_poll is enables
> rq clean is not happening. This is because, in napi_poll() when
> work_done < budget, we do not repoll. At this point, enic has disables rq intr
> and has not called napi_complete. Driver assumes that napi will repoll.
> Which does not happen.
>
> Lot of drivers I have checked return full budget if they want to repoll.
> eg. mlx4_en_poll_rx_cq()
I was referring to the "workdone > budget" condition.
But yes, the fix about busy poll seems needed.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/2] enic: fix rx napi poll return value
2015-01-20 13:16 ` [PATCH net 1/2] enic: fix rx napi poll " Govindarajulu Varadarajan
@ 2015-01-25 6:39 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2015-01-25 6:39 UTC (permalink / raw)
To: _govind; +Cc: netdev, ariel.elior, ssujith, benve
From: Govindarajulu Varadarajan <_govind@gmx.com>
Date: Tue, 20 Jan 2015 18:46:15 +0530
> With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
> is done only when work_done == budget. When we are in busy_poll we return 0 in
> napi_poll. We should return budget.
>
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
2015-01-20 13:16 ` [PATCH net 2/2] bnx2x: fix napi poll return value for repoll Govindarajulu Varadarajan
2015-01-20 14:51 ` Eric Dumazet
@ 2015-01-25 6:40 ` David Miller
1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2015-01-25 6:40 UTC (permalink / raw)
To: _govind; +Cc: netdev, ariel.elior, ssujith, benve
From: Govindarajulu Varadarajan <_govind@gmx.com>
Date: Tue, 20 Jan 2015 18:46:16 +0530
> With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
> is done only when work_done == budget. When in busy_poll is we return 0 in
> napi_poll. We should return budget. Also do not return workdone > budget.
>
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
This seems like it needs some adjustments, please make those corrections
and respin this patch.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-01-25 6:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-20 13:16 [PATCH net 0/2] fix napi return value Govindarajulu Varadarajan
2015-01-20 13:16 ` [PATCH net 1/2] enic: fix rx napi poll " Govindarajulu Varadarajan
2015-01-25 6:39 ` David Miller
2015-01-20 13:16 ` [PATCH net 2/2] bnx2x: fix napi poll return value for repoll Govindarajulu Varadarajan
2015-01-20 14:51 ` Eric Dumazet
2015-01-20 14:54 ` Eric Dumazet
2015-01-20 15:26 ` Govindarajulu Varadarajan
2015-01-20 15:24 ` Govindarajulu Varadarajan
2015-01-20 15:44 ` Eric Dumazet
2015-01-25 6:40 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.