Re: [v2 PATCH 3/7] crypto: acomp - Add request chaining and virtual addresses

[Herbert Xu <herbert@gondor.apana.org.au>]

On Tue, Mar 04, 2025 at 09:59:59PM +0000, Sridhar, Kanchana P wrote:
>
> > +static int acomp_reqchain_finish(struct acomp_req_chain *state,
> > +				 int err, u32 mask)
> > +{
> > +	struct acomp_req *req0 = state->req0;
> > +	struct acomp_req *req = state->cur;
> > +	struct acomp_req *n;
> > +
> > +	acomp_reqchain_virt(state, err);
> 
> Unless I am missing something, this seems to be future-proofing, based
> on the initial checks you've implemented in acomp_do_req_chain().
> 
> > +
> > +	if (req != req0)
> > +		list_add_tail(&req->base.list, &req0->base.list);
> > +
> > +	list_for_each_entry_safe(req, n, &state->head, base.list) {
> > +		list_del_init(&req->base.list);
> > +
> > +		req->base.flags &= mask;
> > +		req->base.complete = acomp_reqchain_done;
> > +		req->base.data = state;
> > +		state->cur = req;
> > +
> > +		if (acomp_request_isvirt(req)) {
> > +			unsigned int slen = req->slen;
> > +			unsigned int dlen = req->dlen;
> > +			const u8 *svirt = req->svirt;
> > +			u8 *dvirt = req->dvirt;
> > +
> > +			state->src = svirt;
> > +			state->dst = dvirt;
> > +
> > +			sg_init_one(&state->ssg, svirt, slen);
> > +			sg_init_one(&state->dsg, dvirt, dlen);
> > +
> > +			acomp_request_set_params(req, &state->ssg,
> > &state->dsg,
> > +						 slen, dlen);
> > +		}
> > +
> > +		err = state->op(req);
> > +
> > +		if (err == -EINPROGRESS) {
> > +			if (!list_empty(&state->head))
> > +				err = -EBUSY;
> > +			goto out;
> > +		}
> > +
> > +		if (err == -EBUSY)
> > +			goto out;
> 
> This is a fully synchronous way of processing the request chain, and
> will not work for iaa_crypto's submit-then-poll-for-completions paradigm,
> essential for us to process the compressions in parallel in hardware.
> Without parallelism, we will not derive the full benefits of IAA.

This function is not for chaining drivers at all.  It's for existing
drivers that do *not* support chaining.

If your driver supports chaining, then it should not come through
acomp_reqchain_finish in the first place.  The acomp_reqchain code
translates chained requests to simple unchained ones for the
existing drivers.  If the driver supports chaining natively, then
it will bypass all this go straight to the driver, where you can do
whatever you want with the chained request.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt