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:   Tue, 19 Nov 2019 17:44:32 -0500
From:   Daniel Jordan <daniel.m.jordan@oracle.com>
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     Daniel Jordan <daniel.m.jordan@oracle.com>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Steffen Klassert <steffen.klassert@secunet.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] padata: Remove broken queue flushing
Message-ID: <20191119224432.vr7lyoaqdzpelszo@ca-dmjordan1.us.oracle.com>
References: <20191119051731.yev6dcsp2znjaagz@gondor.apana.org.au>
 <20191119192405.imfi6q4u3g2zgstc@ca-dmjordan1.us.oracle.com>
 <20191119215345.jr7y47b37ivshwcm@gondor.apana.org.au>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191119215345.jr7y47b37ivshwcm@gondor.apana.org.au>
User-Agent: NeoMutt/20180716
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Wed, Nov 20, 2019 at 05:53:45AM +0800, Herbert Xu wrote:
> On Tue, Nov 19, 2019 at 02:24:05PM -0500, Daniel Jordan wrote:
> >
> > __padata_free unconditionally frees pd, so a padata job might choke on it
> > later.  padata_do_parallel calls seem safe because they use RCU, but it seems
> > possible that a job could call padata_do_serial after the instance is gone.
> 
> Actually __padata_free no longer frees the pd after my patch.

I assume you mean the third patch you recently posted, "crypto: pcrypt - Avoid
deadlock by using per-instance padata queues".  That's true, the problem is
fixed there, and the bug being present in bisection doesn't seem like enough
justification to implement something short-lived just to prevent it.

> We should also mandate that __padata_free can only be called after
> all jobs have completed.  This is already the case with pcrypt.

Makes sense to me, though I don't see how it's enforced now in pcrypt.  I'm not
an async crypto person, but wouldn't unloading the pcrypt module when there are
still outstanding async jobs break this rule?

> In fact we should discuss merging padata into pcrypt.  It's been
> 10 years and not a single user of padata has emerged in the kernel.

Actually, I'm working on an RFC right now to add more users for padata.  It
should be posted in the coming week or two, and I hope it can be part of that
discussion.