Re: [PATCH net-next v3 3/4] crypto: caam: Unembed net_dev structure from qi

[Breno Leitao <leitao@debian.org>]

Hello Jakub,

On Wed, Jul 03, 2024 at 07:45:33PM -0700, Jakub Kicinski wrote:
> On Tue,  2 Jul 2024 11:55:53 -0700 Breno Leitao wrote:

> > @@ -751,10 +766,16 @@ int caam_qi_init(struct platform_device *caam_pdev)
> >  		struct caam_qi_pcpu_priv *priv = per_cpu_ptr(&pcpu_qipriv, i);
> >  		struct caam_napi *caam_napi = &priv->caam_napi;
> >  		struct napi_struct *irqtask = &caam_napi->irqtask;
> > -		struct net_device *net_dev = &priv->net_dev;
> > +		struct net_device *net_dev;
> >  
> > +		net_dev = alloc_netdev_dummy(0);
> > +		if (!net_dev) {
> > +			err = -ENOMEM;
> > +			goto fail;
> 
> free_netdev() doesn't take NULL, free_caam_qi_pcpu_netdev()
> will feed it one if we fail here

Sorry, I am not sure I followed you. Let me ask a clarifying questions:

Do you think that free_netdev() will take NULL ?

If that is the case, that *shouldn't* happen, since I have a cpumask
that tracks the percpu netdev that got allocated, and only free those
percpu-net_device that was properly allocated.

Let me simplify the code to make it easy to understand what I had in
mind:

	int caam_qi_init(struct platform_device *caam_pdev) {
		cpumask_clear(&clean_mask);

		net_dev = alloc_netdev_dummy(0);
		if (!net_dev)
			goto fail;

		cpumask_set_cpu(i, &clean_mask);

	fail:
		free_caam_qi_pcpu_netdev(&clean_mask);
	}

	static void free_caam_qi_pcpu_netdev(const cpumask_t *cpus) {
		for_each_cpu(i, cpus) {
			priv = per_cpu_ptr(&pcpu_qipriv, i);
			free_netdev(priv->net_dev);
		}
	}

So, if alloc_netdev_dummy() fails, then the cpu current cpu will not be
set in `clean_mask`, thus, free_caam_qi_pcpu_netdev() will not free it
later.

Anyway, let me know if I am missing something.