[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170428063039.GB6817@gondor.apana.org.au>
Date: Fri, 28 Apr 2017 14:30:39 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Logan Gunthorpe <logang@...tatee.com>
Cc: linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
intel-gfx@...ts.freedesktop.org, linux-raid@...r.kernel.org,
linux-mmc@...r.kernel.org, linux-nvdimm@...ts.01.org,
linux-scsi@...r.kernel.org, open-iscsi@...glegroups.com,
megaraidlinux.pdl@...adcom.com, sparmaintainer@...sys.com,
devel@...verdev.osuosl.org, target-devel@...r.kernel.org,
netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
dm-devel@...hat.com, Christoph Hellwig <hch@....de>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
"James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
Jens Axboe <axboe@...nel.dk>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dan Williams <dan.j.williams@...el.com>,
Ross Zwisler <ross.zwisler@...ux.intel.com>,
Matthew Wilcox <mawilcox@...rosoft.com>,
Sumit Semwal <sumit.semwal@...aro.org>,
Stephen Bates <sbates@...thlin.com>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2 07/21] crypto: shash, caam: Make use of the new sg_map
helper function
On Thu, Apr 27, 2017 at 09:45:57AM -0600, Logan Gunthorpe wrote:
>
>
> On 26/04/17 09:56 PM, Herbert Xu wrote:
> > On Tue, Apr 25, 2017 at 12:20:54PM -0600, Logan Gunthorpe wrote:
> >> Very straightforward conversion to the new function in the caam driver
> >> and shash library.
> >>
> >> Signed-off-by: Logan Gunthorpe <logang@...tatee.com>
> >> Cc: Herbert Xu <herbert@...dor.apana.org.au>
> >> Cc: "David S. Miller" <davem@...emloft.net>
> >> ---
> >> crypto/shash.c | 9 ++++++---
> >> drivers/crypto/caam/caamalg.c | 8 +++-----
> >> 2 files changed, 9 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/crypto/shash.c b/crypto/shash.c
> >> index 5e31c8d..5914881 100644
> >> --- a/crypto/shash.c
> >> +++ b/crypto/shash.c
> >> @@ -283,10 +283,13 @@ int shash_ahash_digest(struct ahash_request *req, struct shash_desc *desc)
> >> if (nbytes < min(sg->length, ((unsigned int)(PAGE_SIZE)) - offset)) {
> >> void *data;
> >>
> >> - data = kmap_atomic(sg_page(sg));
> >> - err = crypto_shash_digest(desc, data + offset, nbytes,
> >> + data = sg_map(sg, 0, SG_KMAP_ATOMIC);
> >> + if (IS_ERR(data))
> >> + return PTR_ERR(data);
> >> +
> >> + err = crypto_shash_digest(desc, data, nbytes,
> >> req->result);
> >> - kunmap_atomic(data);
> >> + sg_unmap(sg, data, 0, SG_KMAP_ATOMIC);
> >> crypto_yield(desc->flags);
> >> } else
> >> err = crypto_shash_init(desc) ?:
> >
> > Nack. This is an optimisation for the special case of a single
> > SG list entry. In fact in the common case the kmap_atomic should
> > disappear altogether in the no-highmem case. So replacing it
> > with sg_map is not acceptable.
>
> What you seem to have missed is that sg_map is just a thin wrapper
> around kmap_atomic. Perhaps with a future check for a mappable page.
> This change should have zero impact on performance.
You are right. Indeed the existing code looks buggy as they
don't take sg->offset into account when doing the kmap. Could
you send me some patches that fix these problems first so that
they can be easily backported?
Thanks,
--
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Powered by blists - more mailing lists