linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tony Battersby <tonyb@cybernetics.com>
To: LABBE Corentin <clabbe.montjoie@gmail.com>,
	herbert@gondor.apana.org.au, davem@davemloft.net,
	akpm@linux-foundation.org, arnd@arndb.de, axboe@fb.com,
	david.s.gordon@intel.com, martin.petersen@oracle.com,
	robert.jarzmik@free.fr, thomas.lendacky@amd.com
Cc: linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	lee.nipper@gmail.com, yuan.j.kang@gmail.com
Subject: Re: [PATCH v2 5/8] lib: introduce sg_nents_len_chained
Date: Fri, 18 Sep 2015 17:25:47 -0400	[thread overview]
Message-ID: <55FC815B.8020206@cybernetics.com> (raw)
In-Reply-To: <55FC3995.8050600@cybernetics.com>

On 09/18/2015 12:19 PM, Tony Battersby wrote:
> But why do drivers even need this at all?  Here is a typical usage:
>
> int qce_mapsg(struct device *dev, struct scatterlist *sg, int nents,
>           enum dma_data_direction dir, bool chained)
> {
>     int err;
>
>     if (chained) {
>         while (sg) {
>             err = dma_map_sg(dev, sg, 1, dir);
>             if (!err)
>                 return -EFAULT;
>             sg = sg_next(sg);
>         }
>     } else {
>         err = dma_map_sg(dev, sg, nents, dir);
>         if (!err)
>             return -EFAULT;
>     }
>
>     return nents;
> }
>
> Here is another:
>
> static int talitos_map_sg(struct device *dev, struct scatterlist *sg,
>               unsigned int nents, enum dma_data_direction dir,
>               bool chained)
> {
>     if (unlikely(chained))
>         while (sg) {
>             dma_map_sg(dev, sg, 1, dir);
>             sg = sg_next(sg);
>         }
>     else
>         dma_map_sg(dev, sg, nents, dir);
>     return nents;
> }
>
> Can anyone clarify why you can't just use dma_map_sg(dev, sg, nents,
> dir) always?  It should be able to handle chained scatterlists just fine.

After further investigation, it looks like this was a remnant of
scatterwalk_sg_next() which was removed by commit 5be4d4c94b1f ("crypto:
replace scatterwalk_sg_next with sg_next").  Apparently crypto
scatterlists used to be chained differently than normal scatterlists, so
functions like dma_map_sg() would not work on a chained crypto
scatterlist, but that is no longer the case.

So instead of adding a new function sg_nents_len_chained(), a better
cleanup would be:
1) replace the driver-specific functions with calls to sg_nents_for_len()
2) get rid of the "chained" variables
3) always call dma_map_sg()/dma_unmap_sg() for the entire scatterlist
regardless of whether or not the scatterlist is chained

Would someone more familiar with the crypto API please confirm that my
suggestions are correct?

Tony Battersby
Cybernetics

  reply	other threads:[~2015-09-18 21:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-18 12:57 [PATCH v2] crypto: Remove duplicate code of SG helpers functions LABBE Corentin
2015-09-18 12:57 ` [PATCH v2 1/8] crypto: bfin: replace sg_count by sg_nents LABBE Corentin
2015-09-18 12:57 ` [PATCH v2 2/8] crypto: amcc replace get_sg_count by sg_nents_for_len LABBE Corentin
2015-09-18 12:57 ` [PATCH v2 3/8] crypto: sahara: replace sahara_sg_length with sg_nents_for_len LABBE Corentin
2015-09-18 12:57 ` [PATCH v2 4/8] s390: replace zfcp_qdio_sbale_count by sg_nents LABBE Corentin
2015-09-30 13:59   ` Steffen Maier
2015-09-18 12:57 ` [PATCH v2 5/8] lib: introduce sg_nents_len_chained LABBE Corentin
2015-09-18 14:01   ` Tom Lendacky
2015-09-18 16:19   ` Tony Battersby
2015-09-18 21:25     ` Tony Battersby [this message]
2015-09-21 14:19       ` Herbert Xu
2015-09-21 14:59         ` LABBE Corentin
2015-09-18 12:57 ` [PATCH v2 6/8] crypto: talitos: replace sg_count with sg_nents_len_chained LABBE Corentin
2015-09-18 12:57 ` [PATCH v2 7/8] crypto: qce: replace qce_countsg " LABBE Corentin
2015-09-18 12:57 ` [PATCH v2 8/8] crypto: caam: replace __sg_count " LABBE Corentin
2015-09-21 15:09 ` [PATCH v2] crypto: Remove duplicate code of SG helpers functions Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55FC815B.8020206@cybernetics.com \
    --to=tonyb@cybernetics.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=axboe@fb.com \
    --cc=clabbe.montjoie@gmail.com \
    --cc=davem@davemloft.net \
    --cc=david.s.gordon@intel.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=lee.nipper@gmail.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=robert.jarzmik@free.fr \
    --cc=thomas.lendacky@amd.com \
    --cc=yuan.j.kang@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).