RE: [PATCH net v2 3/4] bnxt: make sure xmit_more + errors does not miss doorbells

From: David Laight <David.Laight@ACULAB.COM>
To: 'Jakub Kicinski' <kuba@kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>
Cc: "michael.chan@broadcom.com" <michael.chan@broadcom.com>,
	"huangjw@broadcom.com" <huangjw@broadcom.com>,
	"eddie.wai@broadcom.com" <eddie.wai@broadcom.com>,
	"prashant@broadcom.com" <prashant@broadcom.com>,
	"gospo@broadcom.com" <gospo@broadcom.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"edwin.peer@broadcom.com" <edwin.peer@broadcom.com>
Subject: RE: [PATCH net v2 3/4] bnxt: make sure xmit_more + errors does not miss doorbells
Date: Fri, 13 Aug 2021 08:35:39 +0000	[thread overview]
Message-ID: <0d4efdfc3b394ec2bf411dd8036c259e@AcuMS.aculab.com> (raw)
In-Reply-To: <20210811213749.3276687-4-kuba@kernel.org>

From: Jakub Kicinski
> Sent: 11 August 2021 22:38
> 
> skbs are freed on error and not put on the ring. We may, however,
> be in a situation where we're freeing the last skb of a batch,
> and there is a doorbell ring pending because of xmit_more() being
> true earlier. Make sure we ring the door bell in such situations.
> 
> Since errors are rare don't pay attention to xmit_more() and just
> always flush the pending frames.
> 
...
> +tx_free:
>  	dev_kfree_skb_any(skb);
> +tx_kick_pending:
> +	tx_buf->skb = NULL;
> +	if (txr->kick_pending)
> +		bnxt_txr_db_kick(bp, txr, txr->tx_prod);
>  	return NETDEV_TX_OK;

Is this case actually so unlikely that the 'kick' can be
done unconditionally?
Then all the conditionals can be removed from the hot path.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)