[PATCH] lib: memmove: Use optimised memcpy if possible

[PATCH] lib: memmove: Use optimised memcpy if possible

[PrasannaKumar Muralidharan]

When there is no overlap between src and dst use optimised memcpy if it
is available.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.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.

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.

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

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

[PrasannaKumar Muralidharan]

On 4 October 2017 at 22:26, PrasannaKumar Muralidharan
<prasannatsmkumar@gmail.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@imgtec.com>
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.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.
>
> 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.
>
> 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? As there is
no review comment I am wondering what I missed.

Thanks,
PrasannaKumar

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

[PrasannaKumar Muralidharan]

Hi,

On 4 October 2017 at 22:26, PrasannaKumar Muralidharan
<prasannatsmkumar@gmail.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@imgtec.com>
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.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.
>
> 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.
>
> 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?

Regards,
PrasannaKumar

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

[Dan Carpenter]

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@gmail.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@imgtec.com>
> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.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@imgtec.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@linux-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

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

[PrasannaKumar Muralidharan]

Hi Dan,

On 26 November 2017 at 02:10, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 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.

Okay. I will split his patch into two and send with my modifications.

> On Sat, Nov 25, 2017 at 10:52:04PM +0530, PrasannaKumar Muralidharan wrote:
>> Hi,
>>
>> On 4 October 2017 at 22:26, PrasannaKumar Muralidharan
>> <prasannatsmkumar@gmail.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@imgtec.com>
>> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.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@imgtec.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.

Sure. Will do.

>> >
>> > 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.

I will try to come up with some reasonable benchmark and post its results.

>> > 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@linux-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.

I will add Andrew. Get maintainer script gave me a small list of email
id for this. I don't know if there is a better way than using
get_maintainer.pl.

> regards,
> dan carpenter

Thanks a lot for your time Dan.

Thanks,
PrasannaKumar