On Thu, Sep 05, 2019 at 02:17:35PM +1000, Herbert Xu wrote:
> On Wed, Aug 28, 2019 at 06:14:21PM -0400, Daniel Jordan wrote:
> >
> > @@ -453,24 +456,15 @@ static void padata_free_pd(struct parallel_data *pd)
> > /* Flush all objects out of the padata queues. */
> > static void padata_flush_queues(struct parallel_data *pd)
> > {
> > - int cpu;
> > - struct padata_parallel_queue *pqueue;
> > - struct padata_serial_queue *squeue;
> > -
> > - for_each_cpu(cpu, pd->cpumask.pcpu) {
> > - pqueue = per_cpu_ptr(pd->pqueue, cpu);
> > - flush_work(&pqueue->work);
> > - }
> > -
> > - if (atomic_read(&pd->reorder_objects))
> > - padata_reorder(pd);
> > + if (!(pd->pinst->flags & PADATA_INIT))
> > + return;
> >
> > - for_each_cpu(cpu, pd->cpumask.cbcpu) {
> > - squeue = per_cpu_ptr(pd->squeue, cpu);
> > - flush_work(&squeue->work);
> > - }
> > + if (atomic_dec_return(&pd->refcnt) == 0)
> > + complete(&pd->flushing_done);
> >
> > - BUG_ON(atomic_read(&pd->refcnt) != 0);
> > + wait_for_completion(&pd->flushing_done);
> > + reinit_completion(&pd->flushing_done);
> > + atomic_set(&pd->refcnt, 1);
> > }
>
> I don't think waiting is an option. In a pathological case the
> hardware may not return at all. We cannot and should not hold off
> CPU hotplug for an arbitrary amount of time when the event we are
> waiting for isn't even occuring on that CPU.
Ok, I hadn't considered hardware not returning.
> I don't think flushing is needed at all. All we need to do is
> maintain consistency before and after the CPU hotplug event.
I could imagine not flushing would work for replacing a pd. The old pd could
be freed by whatever drops the last reference and the new pd could be
installed, all without flushing.
In the case of freeing an instance, though, padata needs to wait for all the
jobs to complete so they don't use the instance's data after it's been freed.
Holding the CPU hotplug lock isn't necessary for this, though, so I think we're
ok to wait here.
