Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 22 Nov 2019 11:28:12 -0500
From:   Daniel Jordan <daniel.m.jordan@oracle.com>
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     Eric Biggers <ebiggers@kernel.org>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Steffen Klassert <steffen.klassert@secunet.com>,
        Daniel Jordan <daniel.m.jordan@oracle.com>
Subject: Re: [v2 PATCH] crypto: pcrypt - Avoid deadlock by using per-instance
 padata queues
Message-ID: <20191122162812.a4uzsnvr7crghtzo@ca-dmjordan1.us.oracle.com>
References: <20191119130556.dso2ni6qlks3lr23@gondor.apana.org.au>
 <20191119173732.GB819@sol.localdomain>
 <20191119185827.nerskpvddkcsih25@gondor.apana.org.au>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191119185827.nerskpvddkcsih25@gondor.apana.org.au>
User-Agent: NeoMutt/20180716
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Wed, Nov 20, 2019 at 02:58:27AM +0800, Herbert Xu wrote:
>  static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
>  {
> -	struct parallel_data *pd = NULL;
> -
>  	if (cpumask_test_cpu(cpu, cpu_online_mask)) {
> +		cpumask_clear_cpu(cpu, pinst->cpumask.pcpu);
> +		cpumask_clear_cpu(cpu, pinst->cpumask.cbcpu);
>  
>  		if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) ||
>  		    !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
>  			__padata_stop(pinst);
>  
> -		pd = padata_alloc_pd(pinst, pinst->cpumask.pcpu,
> -				     pinst->cpumask.cbcpu);
> -		if (!pd)
> -			return -ENOMEM;
> -
> -		padata_replace(pinst, pd);
> -
> -		cpumask_clear_cpu(cpu, pd->cpumask.cbcpu);
> -		cpumask_clear_cpu(cpu, pd->cpumask.pcpu);
> +		padata_replace(pinst);
>  	}

Clearing the offlined CPU from pinst's cpumasks means it won't be used if it
comes back online.

The CPU could be cleared from all pd's attached to the instance, but that
doesn't address another bug in this function that's fixed by this patch:

    https://lore.kernel.org/linux-crypto/20190828221425.22701-6-daniel.m.jordan@oracle.com/

Should I add it into the next version of the padata series I just posted?:

    https://lore.kernel.org/linux-crypto/20191120185412.302-1-daniel.m.jordan@oracle.com/

It would obviate the need for your patch to mess with cpumask_clear_cpu().  Or
is there something else you'd prefer?

Thanks,
Daniel