linux-kernel - Re: [PATCH v2 1/5] padata: make flushing work with async users

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190905223756.wmmjkjvztlerjzee@ca-dmjordan1.us.oracle.com>
Date:   Thu, 5 Sep 2019 18:37:56 -0400
From:   Daniel Jordan <daniel.m.jordan@...cle.com>
To:     Herbert Xu <herbert@...dor.apana.org.au>
Cc:     Daniel Jordan <daniel.m.jordan@...cle.com>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/5] padata: make flushing work with async users

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.