Re: [PATCH v3 1/2] driver core: Introduce device_link_wait_removal()

[Herve Codina]

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é