[Linux-ia64] Re: memcpy failure

[Linux-ia64] Re: memcpy failure

[Christian Cotte-Barrot]

"Chen, Kenneth W" wrote:
> 
> The retrun value for memcpy doesn't follow the user space memcpy exactly.
> kernel memcpy always return 0.
> - Ken
> 

But memcpy from memcpy.S is returning a pointer to dest area.
That would lead to quasi non-portable code when the return from memcopy
is correctly checked depending on which memcopy function is addressed.
But BTW, is it meaningful to take into account a return code
that is always the same and which value is known in advance ?
Does memcpy suppose to failed in some cases ?

Re: [Linux-ia64] Re: memcpy failure

[Matthew Wilcox]

On Thu, Nov 14, 2002 at 10:01:44AM +0100, Christian Cotte-Barrot wrote:
> But memcpy from memcpy.S is returning a pointer to dest area.
> That would lead to quasi non-portable code when the return from memcopy
> is correctly checked depending on which memcopy function is addressed.
> But BTW, is it meaningful to take into account a return code
> that is always the same and which value is known in advance ?
> Does memcpy suppose to failed in some cases ?

no, memcpy cannot fail (you get a SIGSEGV in userspace or an MCA in
kernel space).  checking the return value is meaningless.

-- 
Revolutions do not require corporate support.

Re: [Linux-ia64] Re: memcpy failure

[Don Dugger]

I sometimes prefer a routine to return a pointer, even if the
pointer is known in advance.  This way you can set a pointer
in the same statement that does the copy, e.g.:

	ptr = memcpy(malloc(100), buffer, 100);

rather than:

	ptr = malloc(100);
	memcpy(ptr, buffer, 100);

This is just a stylistic issue, I like to remove lines of code
whenever possible.

PS:  I would never actually do this code example, using a malloc
without checking the result is wrong, but you get the idea.

On Thu, Nov 14, 2002 at 10:01:44AM +0100, Christian Cotte-Barrot wrote:
> "Chen, Kenneth W" wrote:
> > 
> > The retrun value for memcpy doesn't follow the user space memcpy exactly.
> > kernel memcpy always return 0.
> > - Ken
> > 
> 
> But memcpy from memcpy.S is returning a pointer to dest area.
> That would lead to quasi non-portable code when the return from memcopy
> is correctly checked depending on which memcopy function is addressed.
> But BTW, is it meaningful to take into account a return code
> that is always the same and which value is known in advance ?
> Does memcpy suppose to failed in some cases ?
> 
> _______________________________________________
> Linux-IA64 mailing list
> Linux-IA64@linuxia64.org
> http://lists.linuxia64.org/lists/listinfo/linux-ia64

-- 
Don Dugger
"Censeo Toto nos in Kansa esse decisse." - D. Gale
n0ano@n0ano.com

[Linux-ia64] RE: memcpy failure

[Chen, Kenneth W]

David,

I was thinking along the same line as returning void for memcpy.  And that is what implemented in memcpy_mck.S. r8 is initialized to zero in the code path mainly for copy_user.

I suppose I should follow the convention since everyone except me expect memcpy to return dest pointer and actually use the return value ;-)  I can work on that although I need to shuffle some instruction around to avoid  expand the code size and worsen the copy throughput.

- Ken


-----Original Message-----
From: David Mosberger [mailto:davidm@napali.hpl.hp.com]
Sent: Wednesday, November 13, 2002 9:52 PM
To: Chen, Kenneth W
Cc: Christian Cotte-Barrot; David Mosberger; Stephane Eranian
Subject: RE: memcpy failure


>>>>> On Tue, 12 Nov 2002 10:29:23 -0800, "Chen, Kenneth W" <kenneth.w.chen@intel.com> said:

  Ken> The retrun value for memcpy doesn't follow the user space
  Ken> memcpy exactly.  kernel memcpy always return 0.

Why are you saying this?  As far as I know, the kernel memcpy() is
expected to return the (initial) destination address.  Perhaps you're
thinking of copy_user()?

(If it was up to me, memcpy() would be returning void...).

	--david

RE: [Linux-ia64] Re: memcpy failure

[Chen, Kenneth W]

> >On Thu, Nov 14, 2002 at 10:01:44AM +0100, Christian Cotte-Barrot wrote:
> > But memcpy from memcpy.S is returning a pointer to dest area.
> > That would lead to quasi non-portable code when the return from memcopy
> > is correctly checked depending on which memcopy function is addressed.
> > But BTW, is it meaningful to take into account a return code
> > that is always the same and which value is known in advance ?
> > Does memcpy suppose to failed in some cases ?

> no, memcpy cannot fail (you get a SIGSEGV in userspace or an MCA in
> kernel space).  checking the return value is meaningless.

That's what I feel as well that it is kind of silly to return something that the caller already has. BUt then, I'm not here to judge the specification ;-)

Anyhow, the fix is fairly easy and won't affect the code size as well as the copy throughput, I should have a patch ready shortly to make everyone happy!

- Ken

Re: [Linux-ia64] Re: memcpy failure

[Mario Smarduch]

Matthew Wilcox wrote:

> On Thu, Nov 14, 2002 at 10:01:44AM +010-, Christian Cotte-Barrot wrote:
> > But memcpy from memcpy.S is returning a pointer to dest area.
> > That would lead to quasi non-portable code when the return from memcopy
> > is correctly checked depending on which memcopy function is addressed.
> > But BTW, is it meaningful to take into account a return code
> > that is always the same and which value is known in advance ?
> > Does memcpy suppose to failed in some cases ?
>
> no, memcpy cannot fail (you get a SIGSEGV in userspace or an MCA in
> kernel space).  checking the return value is meaningless.
>
> --
> Revolutions do not require corporate support.
>
> _______________________________________________
> Linux-IA64 mailing list
> Linux-IA64@linuxia64.org
> http://lists.linuxia64.org/lists/listinfo/linux-ia64

>

Aren't all these routines suppose to return -EFAULT if a fault occurs?

- Mario.

Re: [Linux-ia64] Re: memcpy failure

[Matthew Wilcox]

On Thu, Nov 14, 2002 at 10:09:34AM -0600, Mario Smarduch wrote:
> Aren't all these routines suppose to return -EFAULT if a fault occurs?

No.  Did you go to the Richard B Johnson school of C programming?

-- 
Revolutions do not require corporate support.

Re: [Linux-ia64] Re: memcpy failure

[Mario Smarduch]

Matthew Wilcox wrote:

> On Thu, Nov 14, 2002 at 10:09:34AM -0600, Mario Smarduch wrote:
> > Aren't all these routines suppose to return -EFAULT if a fault occurs?
>
> No.  Did you go to the Richard B Johnson school of C programming?
>
> --
> Revolutions do not require corporate support.

I'm referring to page faults and what handle_exception() does -  regs->r8 -EFAULT

Re: [Linux-ia64] Re: memcpy failure

[David Mosberger]

>>>>> On Thu, 14 Nov 2002 13:21:59 -0600, Mario Smarduch <cms063@email.mot.com> said:

  Mario> I'm referring to page faults and what handle_exception() does
  Mario> - regs->r8 = -EFAULT

In the case of memcpy(), there are no exception regions ("handlers"),
so handle_exception() won't do anything.  copy_user() is different
that way.

	--david

RE: [Linux-ia64] Re: memcpy failure

[Chen, Kenneth W]

[-- Attachment #1: Type: text/plain, Size: 1483 bytes --]

Here is the patch that I promised.

Bjorn, would you please apply it to the 2.4 tree? and
David, would you please apply it to the 2.5 tree?

Thanks.

- Ken


-----Original Message-----
From: Chen, Kenneth W 
Sent: Thursday, November 14, 2002 7:53 AM
To: Matthew Wilcox; Christian Cotte-Barrot
Cc: linux-ia64@linuxia64.org
Subject: RE: [Linux-ia64] Re: memcpy failure



> >On Thu, Nov 14, 2002 at 10:01:44AM +0100, Christian Cotte-Barrot wrote:
> > But memcpy from memcpy.S is returning a pointer to dest area.
> > That would lead to quasi non-portable code when the return from memcopy
> > is correctly checked depending on which memcopy function is addressed.
> > But BTW, is it meaningful to take into account a return code
> > that is always the same and which value is known in advance ?
> > Does memcpy suppose to failed in some cases ?

> no, memcpy cannot fail (you get a SIGSEGV in userspace or an MCA in
> kernel space).  checking the return value is meaningless.

That's what I feel as well that it is kind of silly to return something that the caller already has. BUt then, I'm not here to judge the specification ;-)

Anyhow, the fix is fairly easy and won't affect the code size as well as the copy throughput, I should have a patch ready shortly to make everyone happy!

- Ken

_______________________________________________
Linux-IA64 mailing list
Linux-IA64@linuxia64.org
http://lists.linuxia64.org/lists/listinfo/linux-ia64

[-- Attachment #2: memcpy_mck.fix.patch --]
[-- Type: application/octet-stream, Size: 1177 bytes --]

--- linux/arch/ia64/lib/memcpy_mck.S.orig	Fri Nov  8 18:05:05 2002
+++ linux/arch/ia64/lib/memcpy_mck.S	Thu Nov 14 11:33:53 2002
@@ -6,7 +6,10 @@
  *	in1:	source address
  *	in2:	number of bytes to copy
  * Output:
- * 	0 if success, or number of byte NOT copied if error occurred.
+ *	for bcopy:     return nothing
+ *	for memcpy:    retrun dest
+ * 	for copy_user: return 0 if success,
+ *		       or number of byte NOT copied if error occurred.
  *
  * Copyright (C) 2002 Intel Corp.
  * Copyright (C) 2002 Ken Chen <kenneth.w.chen@intel.com>
@@ -86,6 +89,7 @@
 	and	r28=0x7,in0
 	and	r29=0x7,in1
 	mov	f6=f0
+	mov	retval=in0
 	br.cond.sptk .common_code
 	;;
 END(memcpy)
@@ -97,7 +101,7 @@
 	mov	f6=f1
 	mov	saved_in0=in0	// save dest pointer
 	mov	saved_in1=in1	// save src pointer
-	mov	saved_in2=in2	// save len
+	mov	retval=r0	// initialize return value
 	;;
 .common_code:
 	cmp.gt	p15,p0=8,in2	// check for small size
@@ -105,7 +109,7 @@
 	cmp.ne	p14,p0=0,r29	// check src alignment
 	add	src0=0,in1
 	sub	r30=8,r28	// for .align_dest
-	mov	retval=r0	// initialize return value
+	mov	saved_in2=in2	// save len
 	;;
 	add	dst0=0,in0
 	add	dst1=1,in0	// dest odd index

Re: [Linux-ia64] Re: memcpy failure

[Christian Cotte-Barrot]

"Chen, Kenneth W" wrote:
> 
> > >On Thu, Nov 14, 2002 at 10:01:44AM +0100, Christian Cotte-Barrot wrote:
> > > But memcpy from memcpy.S is returning a pointer to dest area.
> > > That would lead to quasi non-portable code when the return from memcopy
> > > is correctly checked depending on which memcopy function is addressed.
> > > But BTW, is it meaningful to take into account a return code
> > > that is always the same and which value is known in advance ?
> > > Does memcpy suppose to failed in some cases ?
> 
> > no, memcpy cannot fail (you get a SIGSEGV in userspace or an MCA in
> > kernel space).  checking the return value is meaningless.
> 
> That's what I feel as well that it is kind of silly to return something that the caller already has. BUt then, I'm not here to judge the specification ;-)
> 
> Anyhow, the fix is fairly easy and won't affect the code size as well as the copy throughput, I should have a patch ready shortly to make everyone happy!
>

That's too much noise for a thing with slight importance.
It's not a matter of making everyone happy but to provide reliable code.
For e.g. if a device driver checks a return as it must because the
the function is suppose to provide one, the return code in question
must be consistent.
On bad return from memcpy the concerned device driver loggs warning
in /var/log/messages and the system administrator may have some doubt
about the good working of the device.

Thanks for the patch Ken.

Re: [Linux-ia64] Re: memcpy failure

[Matthew Wilcox]

On Fri, Nov 15, 2002 at 11:32:10AM +0100, Christian Cotte-Barrot wrote:
> On bad return from memcpy the concerned device driver loggs warning
> in /var/log/messages and the system administrator may have some doubt
> about the good working of the device.

_what_ bad return from memcpy?!  memcpy returns the pointer that was
passed to it.  What are you going to check for?

-- 
Revolutions do not require corporate support.

Re: [Linux-ia64] Re: memcpy failure

[Christian Cotte-Barrot]

Matthew Wilcox wrote:
> 
> On Fri, Nov 15, 2002 at 11:32:10AM +0100, Christian Cotte-Barrot wrote:
> > On bad return from memcpy the concerned device driver loggs warning
> > in /var/log/messages and the system administrator may have some doubt
> > about the good working of the device.
> 
> _what_ bad return from memcpy?!  memcpy returns the pointer that was
> passed to it.  What are you going to check for?
> 

Simply, bad return is whatever is not equal to the dest pointer:
  if ((memcpy(rptr, ch->rxptr + tail, dataToRead)) != rptr)
    ...

Re: [Linux-ia64] Re: memcpy failure

[Matthew Wilcox]

On Fri, Nov 15, 2002 at 02:51:36PM +0100, Christian Cotte-Barrot wrote:
> Matthew Wilcox wrote:
> > _what_ bad return from memcpy?!  memcpy returns the pointer that was
> > passed to it.  What are you going to check for?
> > 
> 
> Simply, bad return is whatever is not equal to the dest pointer:
>   if ((memcpy(rptr, ch->rxptr + tail, dataToRead)) != rptr)
>     ...

No.  You don't get it.  memcpy returns the pointer that was passed in.
Nothing more, nothing less.  There is no `error return'.  There is no
`what if'.  memcpy is defined to succeed.  Look:

"
7.21.2.1 The memcpy function

Synopsis
#include <string.h>
void *memcpy(void * restrict s1, const void * restrict s2, size_t n);

Description
The memcpy function copies n characters from the object pointed to by s2
into the object pointed to by s1. If copying takes place between objects
that overlap, the behavior is undefined.

Returns
The memcpy function returns the value of s1.
"

Testing the result of memcpy is bad C.  Don't ever do it.

-- 
Revolutions do not require corporate support.