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.
