[Intel-wired-lan] [PATCH 2/2] fm10k: return smaller of actual work done or budget in fm10k_poll

[Keller, Jacob E]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Keller, Jacob E
> Sent: Wednesday, August 10, 2016 12:49 PM
> To: Intel Wired LAN <intel-wired-lan at lists.osuosl.org>; Duyck, Alexander H
> <alexander.h.duyck at intel.com>
> Cc: Paolo Abeni <pabeni at redhat.com>; Venkatesh Srinivas
> <venkateshs at google.com>
> Subject: Re: [Intel-wired-lan] [PATCH 2/2] fm10k: return smaller of actual
> work done or budget in fm10k_poll
> 
> > -----Original Message-----
> > From: Keller, Jacob E
> > Sent: Monday, June 20, 2016 10:40 AM
> > To: Intel Wired LAN <intel-wired-lan at lists.osuosl.org>
> > Cc: Keller, Jacob E <jacob.e.keller at intel.com>; Paolo Abeni
> > <pabeni at redhat.com>; Venkatesh Srinivas <venkateshs at google.com>;
> > Alexander Duyck <aduyck at mirantis.com>
> > Subject: [PATCH 2/2] fm10k: return smaller of actual work done or budget
> in
> > fm10k_poll
> >
> > Correct a slight issue with fm10k_poll returning the full budget even if
> > it didn't fully exhaust it. This occurs if a single ring exhausts its
> > per-ring portion of the budget but the other rings do not. Return the
> > actual amount we used capped at budget to avoid issues with napi core.
> >
> > Change-Id: Ie84de029a62adcfa8af49ccb2dc0b4d9f378ee71
> > Cc: Paolo Abeni <pabeni at redhat.com>
> > Cc: Venkatesh Srinivas <venkateshs at google.com>
> > Cc: Alexander Duyck <aduyck at mirantis.com>
> > ---
> >  drivers/net/ethernet/intel/fm10k/fm10k_main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
> > b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
> > index 79d5093d83d1..c2c7f2ef2152 100644
> > --- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
> > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
> > @@ -1474,9 +1474,9 @@ static int fm10k_poll(struct napi_struct *napi,
> int
> > budget)
> >  			clean_complete = false;
> >  	}
> >
> > -	/* If all work not completed, return budget and keep polling */
> > +	/* If all work not completed, return work done and keep polling */
> >  	if (!clean_complete)
> > -		return budget;
> > +		return min(work_done, budget);
> >
> 
> This patch potentially appears to be caused Tx hangs in the napi routines in
> the stack, when under heavy traffic pressure. It appears that other drivers
> (i40e, and ixgbe) don't do this either, so I am not sure which is correct...
> 

This patch causes false Tx hangs due to potentially returning 0 when work_done is 0 but the Tx rings aren't clean. After some discussion with Alex I believe that the patch is just wrong and should be dropped. Jeff, can you drop this patch from the queue? The other related patch is actually correct and should remain.

Thanks,
Jake