netdev - Re: [PATCH net-next] net: ibm: replenish rx pool and poll less frequently

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4765c54a8cb7b87ae1d7db928c44f40b@imap.linux.ibm.com>
Date:   Wed, 02 Jun 2021 10:58:12 -0700
From:   Dany Madden <drt@...ux.ibm.com>
To:     Lijun Pan <lijunp213@...il.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: ibm: replenish rx pool and poll less
 frequently

On 2021-06-02 10:01, Lijun Pan wrote:
> The old mechanism replenishes rx pool even only one frames is processed 
> in
> the poll function, which causes lots of overheads. The old mechanism
> restarts polling until processed frames reaches the budget, which can
> cause the poll function to loop into restart_poll 63 times at most and 
> to
> call replenish_rx_poll 63 times at most. This will cause soft lockup 
> very
> easily. So, don't replenish too often, and don't goto restart_poll in 
> each
> poll function. If there are pending descriptors, fetch them in the next
> poll instance.

Does this improve performance?

> 
> Signed-off-by: Lijun Pan <lijunp213@...il.com>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index ffb2a91750c7..fae1eaa39dd0 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -2435,7 +2435,6 @@ static int ibmvnic_poll(struct napi_struct
> *napi, int budget)
>  	frames_processed = 0;
>  	rx_scrq = adapter->rx_scrq[scrq_num];
> 
> -restart_poll:
>  	while (frames_processed < budget) {
>  		struct sk_buff *skb;
>  		struct ibmvnic_rx_buff *rx_buff;
> @@ -2512,20 +2511,12 @@ static int ibmvnic_poll(struct napi_struct
> *napi, int budget)
>  	}
> 
>  	if (adapter->state != VNIC_CLOSING &&
> -	    ((atomic_read(&adapter->rx_pool[scrq_num].available) <
> -	      adapter->req_rx_add_entries_per_subcrq / 2) ||
> -	      frames_processed < budget))

There is a budget that the driver should adhere to. Even one frame, it 
should still process the frame within a budget.

> +	    (atomic_read(&adapter->rx_pool[scrq_num].available) <
> +	      adapter->req_rx_add_entries_per_subcrq / 2))
>  		replenish_rx_pool(adapter, &adapter->rx_pool[scrq_num]);
>  	if (frames_processed < budget) {
> -		if (napi_complete_done(napi, frames_processed)) {
> +		if (napi_complete_done(napi, frames_processed))
>  			enable_scrq_irq(adapter, rx_scrq);
> -			if (pending_scrq(adapter, rx_scrq)) {
> -				if (napi_reschedule(napi)) {
> -					disable_scrq_irq(adapter, rx_scrq);
> -					goto restart_poll;
> -				}
> -			}
> -		}
>  	}
>  	return frames_processed;
>  }