Re: [PATCH net-next] sfc: handle VI shortage on ef100 by readjusting the channels

[Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>]

On 25/05/2023 15:51, Edward Cree wrote:
> On 24/05/2023 10:36, Pieter Jansen van Vuuren wrote:
>> When fewer VIs are allocated than what is allowed we can readjust
>> the channels by calling efx_mcdi_alloc_vis() again.
>>
>> Signed-off-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
>> Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com>
> 
> Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>
> though see below for one nit (fix in a follow-up?)
> 
>> ---
>>  drivers/net/ethernet/sfc/ef100_netdev.c | 51 ++++++++++++++++++++++---
>>  1 file changed, 45 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
>> index d916877b5a9a..c201e001f3b8 100644
>> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
>> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
>> @@ -40,19 +40,26 @@ static int ef100_alloc_vis(struct efx_nic *efx, unsigned int *allocated_vis)
>>  	unsigned int tx_vis = efx->n_tx_channels + efx->n_extra_tx_channels;
>>  	unsigned int rx_vis = efx->n_rx_channels;
>>  	unsigned int min_vis, max_vis;
>> +	int rc;
>>  
>>  	EFX_WARN_ON_PARANOID(efx->tx_queues_per_channel != 1);
>>  
>>  	tx_vis += efx->n_xdp_channels * efx->xdp_tx_per_channel;
>>  
>>  	max_vis = max(rx_vis, tx_vis);
>> -	/* Currently don't handle resource starvation and only accept
>> -	 * our maximum needs and no less.
>> +	/* We require at least a single complete TX channel worth of queues. */
>> +	min_vis = efx->tx_queues_per_channel;
>> +
>> +	rc = efx_mcdi_alloc_vis(efx, min_vis, max_vis,
>> +				NULL, allocated_vis);
> 
> I'd like a check here like
>     if (rc == -EAGAIN)
>             rc = -ESOMETHINGELSE;
>  just to avoid confusion if the MC or MCDI machinery returns EAGAIN
>  for whatever reason.
> Or perhaps better still, don't overload EAGAIN like this and instead
>  have this function return 1 in the "we succeeded but didn't get
>  max_vis" case (would need a comment above the function, documenting
>  this), since that's not a value that can happen in-band.
> 
> -ed

Thank you Ed. Yes, I will work with you on the follow-up and then avoid
overloading EAGAIN.