linux-kernel - Re: [PATCH] lib: memmove: Use optimised memcpy if possible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 25 Nov 2017 23:40:05 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>
Cc:     open list <linux-kernel@...r.kernel.org>, mchehab@...nel.org,
        Paul Burton <paul.burton@...tec.com>
Subject: Re: [PATCH] lib: memmove: Use optimised memcpy if possible

Paul's original patch should have been separated into two patches to
begin with.  The patch does two different things and one part goes
through the MIPS tree and one part goes through Andrew, probably.

On Sat, Nov 25, 2017 at 10:52:04PM +0530, PrasannaKumar Muralidharan wrote:
> Hi,
> 
> On 4 October 2017 at 22:26, PrasannaKumar Muralidharan
> <prasannatsmkumar@...il.com> wrote:
> > When there is no overlap between src and dst use optimised memcpy if it
> > is available.
> >
> > Signed-off-by: Paul Burton <paul.burton@...tec.com>
> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>
> > ---
> > This change is a small part of a patch [1] from Paul Burton. I have
> > added his Signed-off by. I do not know whether it is correct. Please let
> > me know if it has to be changed, I will send a v2.

Sign-off is like signing a legal document.  Read
Documentation/process/submitting-patches.rst the section about
"11) Sign your work - the Developer's Certificate of Origin" for an
explanation.

So, yeah, Paul provided his s-o-b and it needs to be here as well.  But
also he maybe should get authorship credit.  Just put the first line in
the email as:

From: Paul Burton <paul.burton@...tec.com>

But that's sort of a trickier thing, so maybe put some explanation that
you chopped out a bit from Pauls patch in the changelog:

	This is part of a patch that Paul Burton wrote
	https://patchwork.linux-mips.org/patch/14517/

I know you put that here, but since it's under the --- cut off it won't
be saved in the final git log.

> >
> > This patch is boot tested with qemu for MIPS architecture by removing
> > mips's memmove routine. This patch does not contain MIPS changes. I
> > will try to find out why [1] was not taken already and figure out what
> > to do.
> >

Instead of boot testing, it would be better if we had a benchmark to
show it helped speed things up.

> > 1. https://patchwork.linux-mips.org/patch/14517/
> >
> >  lib/string.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/lib/string.c b/lib/string.c
> > index 9921dc2..462ab7b 100644
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -825,6 +825,17 @@ void *memmove(void *dest, const void *src, size_t count)
> >         char *tmp;
> >         const char *s;
> >
> > +#ifdef __HAVE_ARCH_MEMCPY
> > +       /* Use optimised memcpy when there is no overlap */
> > +       const char *s_end = src + count;
> > +       const char *d = dest;
> > +       char *d_end = dest + count;
> > +
> > +       s = src;
> > +       if ((d_end <= s) || (s_end <= d))
> > +               return memcpy(dest, src, count);
> > +#endif /* __HAVE_ARCH_MEMCPY */
> > +
> >         if (dest <= src) {
> >                 tmp = dest;
> >                 s = src;
> > --
> > 2.10.0
> >
> 
> Is there anything more that I have to do for this patch?
>

Probably a patch like this needs to go through Andrew.  Send it again
and CC Andrew Morton <akpm@...ux-foundation.org>.  It would be nice if
we could CC a better list than LKML but I don't know which one...  Few
people read LKML.

regards,
dan carpenter