Re: [PATCH v3 04/19] crypto: scatterwalk - add new functions for copying data

[Eric Biggers <ebiggers@kernel.org>]

On Sun, Mar 02, 2025 at 02:40:56PM +0800, Herbert Xu wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > +void memcpy_from_sglist(void *buf, struct scatterlist *sg,
> > +                       unsigned int start, unsigned int nbytes)
> > {
> >        struct scatter_walk walk;
> > -       struct scatterlist tmp[2];
> > 
> > -       if (!nbytes)
> > +       if (unlikely(nbytes == 0)) /* in case sg == NULL */
> >                return;
> > 
> > -       sg = scatterwalk_ffwd(tmp, sg, start);
> > +       scatterwalk_start_at_pos(&walk, sg, start);
> > +       memcpy_from_scatterwalk(buf, &walk, nbytes);
> > +}
> > +EXPORT_SYMBOL_GPL(memcpy_from_sglist);
> > +
> > +void memcpy_to_sglist(struct scatterlist *sg, unsigned int start,
> > +                     const void *buf, unsigned int nbytes)
> 
> These functions duplicate sg_copy_buffer.  Of course scatterwalk
> in general duplicates SG miter which came later IIRC.
> 
> What's your plan for eliminating this duplication?
> 
> Thanks,

The new functions are much better than the lib/scatterlist.c ones: they have a
much better implementation that is faster and doesn't use atomic kmaps, and
(like scatterwalk_map_and_copy() which they are replacing first) they don't
require the unhelpful 'nents' parameter.  My tentative plan is to move them into
lib/scatterlist.c, reimplement sg_copy_buffer() et al on top of them, then
eventually update the callers to use the new functions directly.

However, the 'nents' parameter that sg_copy_buffer() et al take will make the
unification a bit difficult.  Currently those functions copy the minimum of
'buflen' bytes and the first 'nents' scatterlist elements.  I'd like to remove
the 'nents' parameter and just have 'buflen' (or rather 'nbytes'), like the
crypto/scatterwalk.c functions.  I suspect that nearly all callers are passing
in enough 'nents' to cover their 'buflen'.  But there may be some exceptions,
which we'll need to check for.

- Eric