Re: [PATCH net] net: atlantic: keep rings across suspend/resume

[Simon Horman]

On Thu, Dec 12, 2024 at 03:39:24AM +0100, Lorenz Brun wrote:
> The rings are order-6 allocations which tend to fail on suspend due to
> fragmentation. As memory is kept during suspend/resume, we don't need to
> reallocate them.
> 
> This does not touch the PTP rings which, if enabled, still reallocate.
> Fixing these is harder as the whole structure is reinitialized.
> 
> Fixes: cbe6c3a8f8f4 ("net: atlantic: invert deep par in pm functions, preventing null derefs")
> Signed-off-by: Lorenz Brun <lorenz@xxxxxxxx>

...

> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_vec.c b/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
> index 9769ab4f9bef..3b51d6ee0812 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
> @@ -132,6 +132,16 @@ int aq_vec_ring_alloc(struct aq_vec_s *self, struct aq_nic_s *aq_nic,
>  	unsigned int i = 0U;
>  	int err = 0;
>  
> +	if (self && self->tx_rings == aq_nic_cfg->tcs && self->rx_rings == aq_nic_cfg->tcs) {
> +		/* Correct rings already allocated, nothing to do here */
> +		return 0;
> +	} else if (self && (self->tx_rings > 0 || self->rx_rings > 0)) {
> +		/* Allocated rings are different, free rings and reallocate */
> +		pr_notice("%s: cannot reuse rings, have %d, need %d, reallocating", __func__,
> +			  self->tx_rings, aq_nic_cfg->tcs);
> +		aq_vec_ring_free(self);
> +	}
> +

Hi Lorenzo,

Can self be NULL here?

In the for loop below it is dereferenced unconditionally and
thus assumed not to be NULL there.

Flagged by Smatch.

>  	for (i = 0; i < aq_nic_cfg->tcs; ++i) {
>  		const unsigned int idx_ring = AQ_NIC_CFG_TCVEC2RING(aq_nic_cfg,
>  								    i, idx);
> -- 
> 2.44.1
> 
>