Btrfs devel - Sep 2010 - [PATCH 2/3] lib: improve the performance of memcpy and memmove of the general version

the performance of memcpy and memmove of the general version is
very inefficient, this patch improved them.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 lib/string.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index f71bead..6cbf6d8 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -23,6 +23,7 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
 #include <linux/module.h>
+#include <linux/memcopy.h>
 
 #ifndef __HAVE_ARCH_STRNICMP
 /**
@@ -567,11 +568,12 @@ EXPORT_SYMBOL(memset);
  */
 void *memcpy(void *dest, const void *src, size_t count)
 {
-	char *tmp = dest;
-	const char *s = src;
+	unsigned long dstp = (unsigned long)dest;
+	unsigned long srcp = (unsigned long)src;
+
+	/* Copy from the beginning to the end */
+	mem_copy_fwd(dstp, srcp, count);
 
-	while (count--)
-		*tmp++ = *s++;
 	return dest;
 }
 EXPORT_SYMBOL(memcpy);
@@ -588,21 +590,15 @@ EXPORT_SYMBOL(memcpy);
  */
 void *memmove(void *dest, const void *src, size_t count)
 {
-	char *tmp;
-	const char *s;
-
-	if (dest <= src) {
-		tmp = dest;
-		s = src;
-		while (count--)
-			*tmp++ = *s++;
+	unsigned long dstp = (unsigned long)dest;
+	unsigned long srcp = (unsigned long)src;
+
+	if (dest - src >= count) {
+		/* Copy from the beginning to the end */
+		mem_copy_fwd(dstp, srcp, count);
 	} else {
-		tmp = dest;
-		tmp += count;
-		s = src;
-		s += count;
-		while (count--)
-			*--tmp = *--s;
+		/* Copy from the end to the beginning */
+		mem_copy_bwd(dstp, srcp, count);
 	}
 	return dest;
 }
-- 
1.7.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

the performance of memcpy and memmove of the general version is
very inefficient, this patch improved them.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 lib/string.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index f71bead..6cbf6d8 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -23,6 +23,7 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
 #include <linux/module.h>
+#include <linux/memcopy.h>
 
 #ifndef __HAVE_ARCH_STRNICMP
 /**
@@ -567,11 +568,12 @@ EXPORT_SYMBOL(memset);
  */
 void *memcpy(void *dest, const void *src, size_t count)
 {
-	char *tmp = dest;
-	const char *s = src;
+	unsigned long dstp = (unsigned long)dest;
+	unsigned long srcp = (unsigned long)src;
+
+	/* Copy from the beginning to the end */
+	mem_copy_fwd(dstp, srcp, count);
 
-	while (count--)
-		*tmp++ = *s++;
 	return dest;
 }
 EXPORT_SYMBOL(memcpy);
@@ -588,21 +590,15 @@ EXPORT_SYMBOL(memcpy);
  */
 void *memmove(void *dest, const void *src, size_t count)
 {
-	char *tmp;
-	const char *s;
-
-	if (dest <= src) {
-		tmp = dest;
-		s = src;
-		while (count--)
-			*tmp++ = *s++;
+	unsigned long dstp = (unsigned long)dest;
+	unsigned long srcp = (unsigned long)src;
+
+	if (dest - src >= count) {
+		/* Copy from the beginning to the end */
+		mem_copy_fwd(dstp, srcp, count);
 	} else {
-		tmp = dest;
-		tmp += count;
-		s = src;
-		s += count;
-		while (count--)
-			*--tmp = *--s;
+		/* Copy from the end to the beginning */
+		mem_copy_bwd(dstp, srcp, count);
 	}
 	return dest;
 }
-- 
1.7.0.1

* Miao Xie:
>  EXPORT_SYMBOL(memcpy);
I think you need to change that to EXPORT_SYMBOL_GPL, because the code
is now licensed under the GPL, and not the GPL plus kernel exceptions
(whatever they are, but they undoubtly exist), unlike the original
implementation.

-- 
Florian Weimer                <fweimer@bfk.de>
BFK edv-consulting GmbH       http://www.bfk.de/
Kriegsstraße 100              tel: +49-721-96201-1
D-76133 Karlsruhe             fax: +49-721-96201-99

On 2010-09-03, at 05:03, Florian Weimer wrote:
> * Miao Xie:
> 
>> EXPORT_SYMBOL(memcpy);
> 
> I think you need to change that to EXPORT_SYMBOL_GPL, because the code
> is now licensed under the GPL, and not the GPL plus kernel exceptions
> (whatever they are, but they undoubtly exist), unlike the original
> implementation.
Ouch.  That would basically make it impossible to implement a non-GPL module. 
Also, this should only apply to the "lib" version of the memcpy()
routine, not the arch-specific ones that are written in assembly (maybe that
already is true, I''m not sure).

Lustre is GPL, so it isn''t fatal for us, but it definitely seems
draconian, and almost a reason not to include this patch into the kernel.  Also,
given that the original code is LGPL (which allows linking to non-GPL code) this
seems counter to the intent of the original authors.

I suspect there isn''t anything so brilliant in the glibc memcpy() that
it couldn''t be re-implemented without copying the code. 
"implement memcpy with aligned machine-word-sized chunks" would be
enough for anyone to reimplement it without ever having come close to the glibc
code.

Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-ext4"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, 2010-09-03 at 11:03 +0000, Florian Weimer wrote:
> * Miao Xie:
> 
> >  EXPORT_SYMBOL(memcpy);
> 
> I think you need to change that to EXPORT_SYMBOL_GPL, because the code
> is now licensed under the GPL, and not the GPL plus kernel exceptions
> (whatever they are, but they undoubtly exist), unlike the original
> implementation.
I wouldn''t think so - the intent of EXPORT_SYMBOL_GPL is to mark
symbols
that make it obvious that a module was derived from the Linux kernel, as
opposed to some sort of generic driver that was just ported to a new
interface. (It''s not foolproof, it''s more of a warning to
developers.)

If you think of it this way, memcpy is a function defined in the C
standard, there''s absolutely nothing Linux-specific about using it.

Of course, IANAL; and you should probably grab some more opinions on the
matter.

-- 
Calvin Walton <calvin.walton@gmail.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, 04 Sep 2010 08:59:02 -0400
Calvin Walton <calvin.walton@gmail.com> wrote:
> On Fri, 2010-09-03 at 11:03 +0000, Florian Weimer wrote:
> > * Miao Xie:
> > 
> > >  EXPORT_SYMBOL(memcpy);
> > 
> > I think you need to change that to EXPORT_SYMBOL_GPL, because the code
> > is now licensed under the GPL, and not the GPL plus kernel exceptions
> > (whatever they are, but they undoubtly exist), unlike the original
> > implementation.
> 
> I wouldn''t think so - the intent of EXPORT_SYMBOL_GPL is to mark
symbols
> that make it obvious that a module was derived from the Linux kernel, as
> opposed to some sort of generic driver that was just ported to a new
> interface. (It''s not foolproof, it''s more of a warning to
developers.)
EXPORT_SYMBOL_GPL was meant for symbols that were clearly internal
workings. EXPORT_SYMBOL() doesn''t in any way imply or excuse GPL
compliance for any derivative work.

Using the FSF memcpy seems a good technical idea, and it''ll no doubt
liven up the proprietary module makers lawyers as it''ll make the FSF a
party to any infringement disputes 8)

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html