Hi Rafael,
On Thu, 29 Feb 2024 14:10:58 +0100
"Rafael J. Wysocki" <rafael@xxxxxxxxxx> wrote:
...
> > > > > +void device_link_wait_removal(void)
> > > > > +{
> > > > > + /*
> > > > > + * devlink removal jobs are queued in the dedicated work queue.
> > > > > + * To be sure that all removal jobs are terminated, ensure that any
> > > > > + * scheduled work has run to completion.
> > > > > + */
> > > > > + drain_workqueue(device_link_wq);
> > > > > +}
> > > >
> > > > I'm still not convinced we can have a recursive call into devlinks removal
> > > > so I
> > > > do think flush_workqueue() is enough. I will defer to Saravana though...
> > >
> > > AFAICS, the difference betwee flush_workqueue() and drain_workqueue()
> > > is the handling of the case when a given work item can queue up itself
> > > again. This does not happen here.
> >
> >
> > Yeah, that's also my understanding...
>
> Moreover, IIUC this is called after dropping the last reference to the
> device link in question and so after queuing up the link removal work.
> Because that work does not requeue itself, flush_workqueue() is
> sufficient to ensure that the removal work has been completed.
>
> If anyone thinks that it may not be sufficient, please explain to me
> why you think so. Otherwise, don't do stuff to prevent things you
> cannot explain.
I will move to flush_workqueue() in the next iteration.
Thanks for the review and the confirmation on this topic.
Best regards,
Hervé
