Messages in this thread |  | | Date | Wed, 06 May 2026 16:43:28 -0400 | | From | Willem de Bruijn <> | | Subject | Re: [PATCH net 3/3] netdevsim: psp: rcu protect psp_dev reference |
| |
Daniel Zahka wrote: > > On 5/6/26 3:34 PM, Willem de Bruijn wrote: > > Daniel Zahka wrote > >> static ssize_t > >> @@ -228,16 +237,23 @@ nsim_psp_rereg_write(struct file *file, const char __user *data, size_t count, > >> loff_t *ppos) > >> { > >> struct netdevsim *ns = file->private_data; > >> - int err; > >> + struct psp_dev *psd; > >> + ssize_t ret; > >> > >> mutex_lock(&ns->psp.rereg_lock); > >> - __nsim_psp_uninit(ns); > >> + __nsim_psp_uninit(ns, false); > >> + > >> + psd = psp_dev_create(ns->netdev, &nsim_psp_ops, &nsim_psp_caps, ns); > >> + if (IS_ERR(psd)) { > >> + ret = PTR_ERR(psd); > >> + goto out; > >> + } > > Do you want to create the new device first and only delete the old > > state if that succeeds? To avoid a netdevsim in state without dev. > > > >> > >> - ns->psp.dev = psp_dev_create(ns->netdev, &nsim_psp_ops, > >> - &nsim_psp_caps, ns); > >> - err = PTR_ERR_OR_ZERO(ns->psp.dev); > >> + rcu_assign_pointer(ns->psp.dev, psd); > >> + ret = count; > >> +out: > >> mutex_unlock(&ns->psp.rereg_lock); > >> - return err ?: count; > >> + return ret; > >> } > >> > > Unfortunately, the way we have psp_dev_unregister() written, it would > clear out the main_netdev->psp_dev field from the second > psp_dev_create() call. I don't believe there is use case to do this in a > real driver, so I'm not sure we need to change how the create/unregister > paths work.
Thanks for that context.
Reviewed-by: Willem de Bruijn <willemb@google.com>
|  |