IS_ERR Threshold Value

Erik Frederiksen · External Since: Jun 28, 2006 Posts: 2

from include/asm-mips/errno.h
#define EDQUOT 1133 /* Quota exceeded */

I noticed that the errno value for EDQUOT on MIPS is considerably larger
than all others. This can lead to a situation where functions using
ERR_PTR() to return error codes in pointers cannot return this error
code without IS_ERR() thinking that the pointer is valid. In my case,
it caused an alignment exception in the XFS open call when quota has
been exceeded in the linux-mips 2.6.14 kernel. I think that the XFS
code has changed enough that this bug isn't in newer versions, though I
haven't done a thorough investigation.

I've supplied a patch that addresses this situation by changing the
threshold used by IS_ERR if EMAXERRNO is defined and greater than 1000.
Perhaps permanently raising the threshold value to something >1133 is
sufficient.

Looking forward to your feedback.

Erik Frederiksen
Firmware Design Engineer Co-op
PMC-Sierra Saskatoon

diff -Nau [ab]/include/linux/err.h
--- a/include/linux/err.h 2005-10-30 13:14:22.000000000 -0600
+++ b/include/linux/err.h 2006-06-28 10:38:43.000000000 -0600
@@ -12,8 +12,23 @@
*
* This should be a per-architecture thing, to allow different
* error and pointer decisions.
+ *
+ * Updated by Erik Frederiksen (erik_frederiksen@pmc-sierra.com)
+ * errno values on MIPS go up to 1133 for EDQUOT. The threshold
+ * is adjusted so that returning large errnos in a pointer
+ * does not result in a valid pointer according to IS_ERR.
*/
-#define IS_ERR_VALUE(x) unlikely((x) > (unsigned long)-1000L)
+
+#define ERR_PTR_THRESHOLD 1000
+#define IS_ERR_VALUE(x) \
+ unlikely((x) > (unsigned long)-(long)ERR_PTR_THRESHOLD )
+#ifdef EMAXERRNO
+# if EMAXERRNO >= ERR_PTR_THRESHOLD
+# undef IS_ERR_VALUE
+# define IS_ERR_VALUE(x) \
+ unlikely((x) >= (unsigned long)-(long)EMAXERRNO )
+# endif
+#endif

static inline void *ERR_PTR(long error)
{

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Randy.Dunlap · External Since: May 15, 2006 Posts: 372

On Wed, 28 Jun 2006 14:57:07 -0600 Erik Frederiksen wrote:

>
> from include/asm-mips/errno.h
> #define EDQUOT 1133 /* Quota exceeded */
>
> I noticed that the errno value for EDQUOT on MIPS is considerably larger
> than all others. This can lead to a situation where functions using
> ERR_PTR() to return error codes in pointers cannot return this error
> code without IS_ERR() thinking that the pointer is valid. In my case,
> it caused an alignment exception in the XFS open call when quota has
> been exceeded in the linux-mips 2.6.14 kernel. I think that the XFS
> code has changed enough that this bug isn't in newer versions, though I
> haven't done a thorough investigation.
>
> I've supplied a patch that addresses this situation by changing the
> threshold used by IS_ERR if EMAXERRNO is defined and greater than 1000.
> Perhaps permanently raising the threshold value to something >1133 is
> sufficient.
>
> Looking forward to your feedback.
>
> Erik Frederiksen
> Firmware Design Engineer Co-op
> PMC-Sierra Saskatoon

Hi,
Peter Anvin mentioned just a few days ago that this threshold value
should be 4095 for all arches. I think we need to get that patch
done & submitted to Andrew for -mm.

> diff -Nau [ab]/include/linux/err.h
> --- a/include/linux/err.h 2005-10-30 13:14:22.000000000 -0600
> +++ b/include/linux/err.h 2006-06-28 10:38:43.000000000 -0600
> @@ -12,8 +12,23 @@
> *
> * This should be a per-architecture thing, to allow different
> * error and pointer decisions.
> + *
> + * Updated by Erik Frederiksen (erik_frederiksen@pmc-sierra.com)
> + * errno values on MIPS go up to 1133 for EDQUOT. The threshold
> + * is adjusted so that returning large errnos in a pointer
> + * does not result in a valid pointer according to IS_ERR.
> */
> -#define IS_ERR_VALUE(x) unlikely((x) > (unsigned long)-1000L)
> +
> +#define ERR_PTR_THRESHOLD 1000
> +#define IS_ERR_VALUE(x) \
> + unlikely((x) > (unsigned long)-(long)ERR_PTR_THRESHOLD )
> +#ifdef EMAXERRNO
> +# if EMAXERRNO >= ERR_PTR_THRESHOLD
> +# undef IS_ERR_VALUE
> +# define IS_ERR_VALUE(x) \
> + unlikely((x) >= (unsigned long)-(long)EMAXERRNO )
> +# endif
> +#endif
>
> static inline void *ERR_PTR(long error)
> {
>
>
> -

---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

H. Peter Anvin · External Since: Feb 08, 2006 Posts: 988

Randy.Dunlap wrote:
> On Wed, 28 Jun 2006 14:57:07 -0600 Erik Frederiksen wrote:
>
>> from include/asm-mips/errno.h
>> #define EDQUOT 1133 /* Quota exceeded */
>>
>> I noticed that the errno value for EDQUOT on MIPS is considerably larger
>> than all others. This can lead to a situation where functions using
>> ERR_PTR() to return error codes in pointers cannot return this error
>> code without IS_ERR() thinking that the pointer is valid. In my case,
>> it caused an alignment exception in the XFS open call when quota has
>> been exceeded in the linux-mips 2.6.14 kernel. I think that the XFS
>> code has changed enough that this bug isn't in newer versions, though I
>> haven't done a thorough investigation.
>>
>> I've supplied a patch that addresses this situation by changing the
>> threshold used by IS_ERR if EMAXERRNO is defined and greater than 1000.
>> Perhaps permanently raising the threshold value to something >1133 is
>> sufficient.
>>
>> Looking forward to your feedback.
>>
>> Erik Frederiksen
>> Firmware Design Engineer Co-op
>> PMC-Sierra Saskatoon
>
> Hi,
> Peter Anvin mentioned just a few days ago that this threshold value
> should be 4095 for all arches. I think we need to get that patch
> done & submitted to Andrew for -mm.
>

Indeed.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Nathan Scott · External Since: Jan 10, 2005 Posts: 115

Hi Erik,

On Wed, Jun 28, 2006 at 02:57:07PM -0600, Erik Frederiksen wrote:
>
> from include/asm-mips/errno.h
> #define EDQUOT 1133 /* Quota exceeded */
>
> I noticed that the errno value for EDQUOT on MIPS is considerably larger
> than all others. This can lead to a situation where functions using
> ERR_PTR() to return error codes in pointers cannot return this error
> code without IS_ERR() thinking that the pointer is valid. In my case,
> it caused an alignment exception in the XFS open call when quota has
> been exceeded in the linux-mips 2.6.14 kernel. I think that the XFS
> code has changed enough that this bug isn't in newer versions, though I
> haven't done a thorough investigation.

Hmm, I'm not sure I understand the XFS side of your report here - on
open, for quota to be coming into play we must be creating a new inode
and those code paths inside XFS have no use of IS_ERR/ERR_PTR magic...
did you mean there's generic problems here (I can see those macros are
used in the generic VFS open() code) ... or am I missing your point?

thanks.

--
Nathan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Erik Frederiksen · External Since: Jun 28, 2006 Posts: 2

Hi Nathan,

On Wed, 2006-06-28 at 16:41, Nathan Scott wrote:
>
> Hmm, I'm not sure I understand the XFS side of your report here - on
> open, for quota to be coming into play we must be creating a new inode
> and those code paths inside XFS have no use of IS_ERR/ERR_PTR magic...
> did you mean there's generic problems here (I can see those macros are
> used in the generic VFS open() code) ... or am I missing your point?

Yes, that's right. The error is being returned from xfs_create when
quota has been exceeded. It ends up carrying back to the filp_open call
in do_sys_open, which returns it as a pointer to a filp structure.
Because the errno is so large, IS_ERR reports it as being a valid
pointer incorrectly.

XFS has acted correctly. The only reason I bring it up is this is how
the bug was brought to my attention.

If there won't be any strange side effects (I don't have the experience
to accurately comment on this), I think turning the threshold value up
to something we can get away with in IS_ERR_VALUE() would be
appropriate.

-erik

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Randy.Dunlap · External Since: May 15, 2006 Posts: 372

On Wed, 28 Jun 2006 17:13:33 -0600 Erik Frederiksen wrote:

> Hi Nathan,
>
> On Wed, 2006-06-28 at 16:41, Nathan Scott wrote:
> >
> > Hmm, I'm not sure I understand the XFS side of your report here - on
> > open, for quota to be coming into play we must be creating a new inode
> > and those code paths inside XFS have no use of IS_ERR/ERR_PTR magic...
> > did you mean there's generic problems here (I can see those macros are
> > used in the generic VFS open() code) ... or am I missing your point?
>
> Yes, that's right. The error is being returned from xfs_create when
> quota has been exceeded. It ends up carrying back to the filp_open call
> in do_sys_open, which returns it as a pointer to a filp structure.
> Because the errno is so large, IS_ERR reports it as being a valid
> pointer incorrectly.
>
> XFS has acted correctly. The only reason I bring it up is this is how
> the bug was brought to my attention.
>
> If there won't be any strange side effects (I don't have the experience
> to accurately comment on this), I think turning the threshold value up
> to something we can get away with in IS_ERR_VALUE() would be
> appropriate.

We need to get the threshold == 4095 patch into -mm to make sure
that it doesn't break anything and/or fix whatever it breaks.

Are you planning to do that patch? If not, someone else (or I) can.

---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Nathan Scott · External Since: Jan 10, 2005 Posts: 115

On Wed, Jun 28, 2006 at 05:13:33PM -0600, Erik Frederiksen wrote:
> On Wed, 2006-06-28 at 16:41, Nathan Scott wrote:
> > Hmm, I'm not sure I understand the XFS side of your report here - on
> ...
> XFS has acted correctly. The only reason I bring it up is this is how
> the bug was brought to my attention.

Ah, OK. When you said this...

| it caused an alignment exception in the XFS open call when quota has
| been exceeded in the linux-mips 2.6.14 kernel. I think that the XFS
| code has changed enough that this bug isn't in newer versions, though I
| haven't done a thorough investigation.

.... I couldn't think of anything we'd changed in XFS that would have
addressed this since that kernel version.

> If there won't be any strange side effects (I don't have the experience
> to accurately comment on this), I think turning the threshold value up
> to something we can get away with in IS_ERR_VALUE() would be
> appropriate.

Seems right to me too, FWIW.

cheers.

--
Nathan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ralf Baechle · External Since: Apr 27, 2006 Posts: 150

On Wed, Jun 28, 2006 at 02:08:25PM -0700, Randy.Dunlap wrote:

> Hi,
> Peter Anvin mentioned just a few days ago that this threshold value
> should be 4095 for all arches. I think we need to get that patch
> done & submitted to Andrew for -mm.

So here the patch is:

o Raise the maximum error number in IS_ERR_VALUE to 4095.
o Make that number available as a new constant MAX_ERRNO.

Signed-off-by: Ralf Baechle <ralf RemoveThis @linux-mips.org>

diff --git a/include/linux/err.h b/include/linux/err.h
index ff71d2a..cd3b367 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -13,7 +13,9 @@ #include <asm/errno.h>
* This should be a per-architecture thing, to allow different
* error and pointer decisions.
*/
-#define IS_ERR_VALUE(x) unlikely((x) > (unsigned long)-1000L)
+#define MAX_ERRNO 4095
+
+#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)

static inline void *ERR_PTR(long error)
{
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Randy.Dunlap · External Since: May 15, 2006 Posts: 372

On Thu, 29 Jun 2006 19:10:13 +0100 Ralf Baechle wrote:

> On Wed, Jun 28, 2006 at 02:08:25PM -0700, Randy.Dunlap wrote:
>
> > Hi,
> > Peter Anvin mentioned just a few days ago that this threshold value
> > should be 4095 for all arches. I think we need to get that patch
> > done & submitted to Andrew for -mm.
>
> So here the patch is:
>
> o Raise the maximum error number in IS_ERR_VALUE to 4095.
> o Make that number available as a new constant MAX_ERRNO.
>
> Signed-off-by: Ralf Baechle <ralf.DeleteThis@linux-mips.org>
>
> diff --git a/include/linux/err.h b/include/linux/err.h
> index ff71d2a..cd3b367 100644
> --- a/include/linux/err.h
> +++ b/include/linux/err.h
> @@ -13,7 +13,9 @@ #include <asm/errno.h>
> * This should be a per-architecture thing, to allow different
> * error and pointer decisions.
> */
> -#define IS_ERR_VALUE(x) unlikely((x) > (unsigned long)-1000L)
> +#define MAX_ERRNO 4095
> +
> +#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
>
> static inline void *ERR_PTR(long error)
> {

Are changes also needed in asm-*/unistd.h::syscall_return() macros?
or is syscall_return() just not used?

e.g.,
arm26 uses -125 to detect error
arm uses -129 to detect error
frv uses -4095 to detect error
i386 uses -129
h8300, m32r, s390, sh64, v850 use -125
m68k[nommu] uses -125
sh uses -124
x86_64 uses -127

---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

H. Peter Anvin · External Since: Feb 08, 2006 Posts: 988

Randy.Dunlap wrote:

>
> Are changes also needed in asm-*/unistd.h::syscall_return() macros?
> or is syscall_return() just not used?
>
> e.g.,
> arm26 uses -125 to detect error
> arm uses -129 to detect error
> frv uses -4095 to detect error
> i386 uses -129
> h8300, m32r, s390, sh64, v850 use -125
> m68k[nommu] uses -125
> sh uses -124
> x86_64 uses -127
>

.... and they're pretty much all wrong (in some cases, they're actually
less than actual errno values on that architecture!)

It pretty much works because they're not used. They should either be
fixed or removed.

-hpa

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ralf Baechle · External Since: Apr 27, 2006 Posts: 150

So MAX_ERRNO of EMAXERRNO which was also being used in assembler code.
Other architectures may have the same issue, so I propose wrapping the
C parts with #ifndef __ASSEMBLY__ to keep as happy.

Signed-off-by: Ralf Baechle <ralf.RemoveThis@linux-mips.org>

diff --git a/include/linux/err.h b/include/linux/err.h
index cd3b367..1ab1d44 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -15,6 +15,8 @@ #include <asm/errno.h>
*/
#define MAX_ERRNO 4095

+#ifndef __ASSEMBLY__
+
#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)

static inline void *ERR_PTR(long error)
@@ -32,4 +34,6 @@ static inline long IS_ERR(const void *pt
return IS_ERR_VALUE((unsigned long)ptr);
}

+#endif
+
#endif /* _LINUX_ERR_H */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

H. Peter Anvin · External Since: Feb 08, 2006 Posts: 988

Ralf Baechle wrote:
> So MAX_ERRNO of EMAXERRNO which was also being used in assembler code.
> Other architectures may have the same issue, so I propose wrapping the
> C parts with #ifndef __ASSEMBLY__ to keep as happy.
>

Indeed; this should definitely be accessible to assembly code. I'd like
to change the hard-coded constants in klibc over time to use this.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Randy.Dunlap · External Since: May 15, 2006 Posts: 372

On Sun, 2 Jul 2006 17:15:20 +0100 Ralf Baechle wrote:

> So MAX_ERRNO of EMAXERRNO which was also being used in assembler code.
> Other architectures may have the same issue, so I propose wrapping the
> C parts with #ifndef __ASSEMBLY__ to keep as happy.

Ack, fixes 'as' whinging for me.

> Signed-off-by: Ralf Baechle <ralf DeleteThis @linux-mips.org>
>
> diff --git a/include/linux/err.h b/include/linux/err.h
> index cd3b367..1ab1d44 100644
> --- a/include/linux/err.h
> +++ b/include/linux/err.h
> @@ -15,6 +15,8 @@ #include <asm/errno.h>
> */
> #define MAX_ERRNO 4095
>
> +#ifndef __ASSEMBLY__
> +
> #define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
>
> static inline void *ERR_PTR(long error)
> @@ -32,4 +34,6 @@ static inline long IS_ERR(const void *pt
> return IS_ERR_VALUE((unsigned long)ptr);
> }
>
> +#endif
> +
> #endif /* _LINUX_ERR_H */
> -

---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Randy.Dunlap · External Since: May 15, 2006 Posts: 372

On Sat, 01 Jul 2006 15:23:31 -0700 H. Peter Anvin wrote:

> > Are changes also needed in asm-*/unistd.h::syscall_return() macros?
> > or is syscall_return() just not used?
> >
> > e.g.,
> > arm26 uses -125 to detect error
> > arm uses -129 to detect error
> > frv uses -4095 to detect error
> > i386 uses -129
> > h8300, m32r, s390, sh64, v850 use -125
> > m68k[nommu] uses -125
> > sh uses -124
> > x86_64 uses -127
> >
>
> ... and they're pretty much all wrong (in some cases, they're actually
> less than actual errno values on that architecture!)
>
> It pretty much works because they're not used. They should either be
> fixed or removed.

From: Randy Dunlap <rdunlap.RemoveThis@xenotime.net>

Consistently use MAX_ERRNO when checking for errors
in __syscall_return().

Signed-off-by: Randy Dunlap <rdunlap.RemoveThis@xenotime.net>
---
include/asm-arm/unistd.h | 3 ++-
include/asm-arm26/unistd.h | 3 ++-
include/asm-frv/unistd.h | 3 ++-
include/asm-h8300/unistd.h | 6 +++---
include/asm-i386/unistd.h | 5 +++--
include/asm-m32r/unistd.h | 5 +++--
include/asm-m68k/unistd.h | 5 +++--
include/asm-m68knommu/unistd.h | 5 +++--
include/asm-s390/unistd.h | 4 +++-
include/asm-sh/unistd.h | 7 +++++--
include/asm-sh64/unistd.h | 6 ++++--
include/asm-v850/unistd.h | 5 +++--
include/asm-x86_64/unistd.h | 5 +++--
13 files changed, 39 insertions(+), 23 deletions(-)

--- linux-2617-g20.orig/include/asm-arm/unistd.h
+++ linux-2617-g20/include/asm-arm/unistd.h
@@ -377,6 +377,7 @@
#endif

#ifdef __KERNEL__
+#include <linux/err.h>
#include <linux/linkage.h>

#define __sys2(x) #x
@@ -396,7 +397,7 @@

#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-129)) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
errno = -(res); \
res = -1; \
} \
--- linux-2617-g20.orig/include/asm-arm26/unistd.h
+++ linux-2617-g20/include/asm-arm26/unistd.h
@@ -311,6 +311,7 @@
#define __ARM_NR_usr26 (__ARM_NR_BASE+3)

#ifdef __KERNEL__
+#include <linux/err.h>
#include <linux/linkage.h>

#define __sys2(x) #x
@@ -322,7 +323,7 @@

#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-125)) { \
+ if ((unsigned long)(res) >= (unsigned long)-MAX_ERRNO) { \
errno = -(res); \
res = -1; \
} \
--- linux-2617-g20.orig/include/asm-frv/unistd.h
+++ linux-2617-g20/include/asm-frv/unistd.h
@@ -320,6 +320,7 @@
#ifdef __KERNEL__

#define NR_syscalls 310
+#include <linux/err.h>

/*
* process the return value of a syscall, consigning it to one of two possible fates
@@ -329,7 +330,7 @@
#define __syscall_return(type, res) \
do { \
unsigned long __sr2 = (res); \
- if (__builtin_expect(__sr2 >= (unsigned long)(-4095), 0)) { \
+ if (__builtin_expect(__sr2 >= (unsigned long)(-MAX_ERRNO), 0)) { \
errno = (-__sr2); \
__sr2 = ~0UL; \
} \
--- linux-2617-g20.orig/include/asm-h8300/unistd.h
+++ linux-2617-g20/include/asm-h8300/unistd.h
@@ -295,14 +295,14 @@
#ifdef __KERNEL__

#define NR_syscalls 289
+#include <linux/err.h>

-
-/* user-visible error numbers are in the range -1 - -122: see
+/* user-visible error numbers are in the range -1 - -MAX_ERRNO: see
<asm-m68k/errno.h> */

#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-125)) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
/* avoid using res which is declared to be in register d0; \
errno might expand to a function call and clobber it. */ \
int __err = -(res); \
--- linux-2617-g20.orig/include/asm-i386/unistd.h
+++ linux-2617-g20/include/asm-i386/unistd.h
@@ -327,14 +327,15 @@
#ifdef __KERNEL__

#define NR_syscalls 318
+#include <linux/err.h>

/*
- * user-visible error numbers are in the range -1 - -128: see
+ * user-visible error numbers are in the range -1 - -MAX_ERRNO: see
* <asm-i386/errno.h>
*/
#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-(128 + 1))) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
errno = -(res); \
res = -1; \
} \
--- linux-2617-g20.orig/include/asm-m32r/unistd.h
+++ linux-2617-g20/include/asm-m32r/unistd.h
@@ -298,14 +298,15 @@
#ifdef __KERNEL__

#define NR_syscalls 285
+#include <linux/err.h>

-/* user-visible error numbers are in the range -1 - -124: see
+/* user-visible error numbers are in the range -1 - -MAX_ERRNO: see
* <asm-m32r/errno.h>
*/

#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-(124 + 1))) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
/* Avoid using "res" which is declared to be in register r0; \
errno might expand to a function call and clobber it. */ \
int __err = -(res); \
--- linux-2617-g20.orig/include/asm-m68k/unistd.h
+++ linux-2617-g20/include/asm-m68k/unistd.h
@@ -288,13 +288,14 @@
#ifdef __KERNEL__

#define NR_syscalls 282
+#include <linux/err.h>

-/* user-visible error numbers are in the range -1 - -124: see
+/* user-visible error numbers are in the range -1 - -MAX_ERRNO: see
<asm-m68k/errno.h> */

#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-125)) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
/* avoid using res which is declared to be in register d0; \
errno might expand to a function call and clobber it. */ \
int __err = -(res); \
--- linux-2617-g20.orig/include/asm-m68knommu/unistd.h
+++ linux-2617-g20/include/asm-m68knommu/unistd.h
@@ -289,13 +289,14 @@
#ifdef __KERNEL__

#define NR_syscalls 282
+#include <linux/err.h>

-/* user-visible error numbers are in the range -1 - -122: see
+/* user-visible error numbers are in the range -1 - -MAX_ERRNO: see
<asm-m68k/errno.h> */

#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-125)) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
/* avoid using res which is declared to be in register d0; \
errno might expand to a function call and clobber it. */ \
int __err = -(res); \
--- linux-2617-g20.orig/include/asm-s390/unistd.h
+++ linux-2617-g20/include/asm-s390/unistd.h
@@ -394,9 +394,11 @@

#ifdef __KERNEL__

+#include <linux/err.h>
+
#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-4095)) {\
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
errno = -(res); \
res = -1; \
} \
--- linux-2617-g20.orig/include/asm-sh64/unistd.h
+++ linux-2617-g20/include/asm-sh64/unistd.h
@@ -347,8 +347,10 @@
#ifdef __KERNEL__

#define NR_syscalls 321
+#include <linux/err.h>

-/* user-visible error numbers are in the range -1 - -125: see <asm-sh64/errno.h> */
+/* user-visible error numbers are in the range -1 - -MAX_ERRNO:
+ * see <asm-sh64/errno.h> */

#define __syscall_return(type, res) \
do { \
@@ -358,7 +360,7 @@ do { \
** life easier in the system call epilogue (see entry.S) \
*/ \
register unsigned long __sr2 __asm__ ("r2") = res; \
- if ((unsigned long)(res) >= (unsigned long)(-125)) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
errno = -(res); \
__sr2 = -1; \
} \
--- linux-2617-g20.orig/include/asm-sh/unistd.h
+++ linux-2617-g20/include/asm-sh/unistd.h
@@ -306,11 +306,14 @@

#ifdef __KERNEL__

-/* user-visible error numbers are in the range -1 - -124: see <asm-sh/errno.h> */
+#include <linux/err.h>
+
+/* user-visible error numbers are in the range -1 - -MAX_ERRNO:
+ * see <asm-sh/errno.h> */

#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-124)) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
/* Avoid using "res" which is declared to be in register r0; \
errno might expand to a function call and clobber it. */ \
int __err = -(res); \
--- linux-2617-g20.orig/include/asm-v850/unistd.h
+++ linux-2617-g20/include/asm-v850/unistd.h
@@ -238,12 +238,13 @@
#ifdef __KERNEL__

#include <asm/clinkage.h>
+#include <linux/err.h>

#define __syscall_return(type, res) \
do { \
- /* user-visible error numbers are in the range -1 - -124: \
+ /* user-visible error numbers are in the range -1 - -MAX_ERRNO: \
see <asm-v850/errno.h> */ \
- if (__builtin_expect ((unsigned long)(res) >= (unsigned long)(-125), 0)) { \
+ if (__builtin_expect ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO), 0)) { \
errno = -(res); \
res = -1; \
} \
--- linux-2617-g20.orig/include/asm-x86_64/unistd.h
+++ linux-2617-g20/include/asm-x86_64/unistd.h
@@ -623,16 +623,17 @@ __SYSCALL(__NR_move_pages, sys_move_page
#ifdef __KERNEL__

#define __NR_syscall_max __NR_move_pages
+#include <linux/err.h>

#ifndef __NO_STUBS

-/* user-visible error numbers are in the range -1 - -4095 */
+/* user-visible error numbers are in the range -1 - -MAX_ERRNO */

#define __syscall_clobber "r11","rcx","memory"

#define __syscall_return(type, res) \
do { \
- if ((unsigned long)(res) >= (unsigned long)(-127)) { \
+ if ((unsigned long)(res) >= (unsigned long)(-MAX_ERRNO)) { \
errno = -(res); \
res = -1; \
} \

---
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Andrew Morton · External Since: Aug 22, 2004 Posts: 2442

On Sun, 2 Jul 2006 11:27:22 -0700
"Randy.Dunlap" <rdunlap.DeleteThis@xenotime.net> wrote:

> --- linux-2617-g20.orig/include/asm-i386/unistd.h
> +++ linux-2617-g20/include/asm-i386/unistd.h
> @@ -327,14 +327,15 @@
> #ifdef __KERNEL__
>
> #define NR_syscalls 318
> +#include <linux/err.h>

include/linux/err.h: Assembler messages:
include/linux/err.h:20: Error: no such instruction: `static inline void *ERR_PTR(long error)'
include/linux/err.h:21: Error: junk at end of line, first unrecognized character is `{'
include/linux/err.h:22: Error: no such instruction: `return (void *)error'
include/linux/err.h:23: Error: junk at end of line, first unrecognized character is `}'
include/linux/err.h:25: Error: no such instruction: `static inline long PTR_ERR(const void *ptr)'
include/linux/err.h:26: Error: junk at end of line, first unrecognized character is `{'
include/linux/err.h:27: Error: no such instruction: `return (long)ptr'
include/linux/err.h:28: Error: junk at end of line, first unrecognized character is `}'
include/linux/err.h:30: Error: no such instruction: `static inline long IS_ERR(const void *ptr)'
include/linux/err.h:31: Error: junk at end of line, first unrecognized character is `{'
include/linux/err.h:32: Error: no such instruction: `return unlikely(((unsigned long)ptr)>=(unsigned long)-4095)'
include/linux/err.h:33: Error: junk at end of line, first unrecognized character is `}'
distcc[7619] ERROR: compile (null) on localhost failed
make[1]: *** [arch/i386/kernel/vsyscall-sysenter.o] Error 1
make: *** [arch/i386/kernel/vsyscall-sysenter.o] Error 2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

H. Peter Anvin · External Since: Feb 08, 2006 Posts: 988

Andrew Morton wrote:
> On Sun, 2 Jul 2006 11:27:22 -0700
> "Randy.Dunlap" <rdunlap.TakeThisOut@xenotime.net> wrote:
>
>> --- linux-2617-g20.orig/include/asm-i386/unistd.h
>> +++ linux-2617-g20/include/asm-i386/unistd.h
>> @@ -327,14 +327,15 @@
>> #ifdef __KERNEL__
>>
>> #define NR_syscalls 318
>> +#include <linux/err.h>
>
> include/linux/err.h: Assembler messages:
> include/linux/err.h:20: Error: no such instruction: `static inline void *ERR_PTR(long error)'
> include/linux/err.h:21: Error: junk at end of line, first unrecognized character is `{'
> include/linux/err.h:22: Error: no such instruction: `return (void *)error'
> include/linux/err.h:23: Error: junk at end of line, first unrecognized character is `}'
> include/linux/err.h:25: Error: no such instruction: `static inline long PTR_ERR(const void *ptr)'
> include/linux/err.h:26: Error: junk at end of line, first unrecognized character is `{'
> include/linux/err.h:27: Error: no such instruction: `return (long)ptr'
> include/linux/err.h:28: Error: junk at end of line, first unrecognized character is `}'
> include/linux/err.h:30: Error: no such instruction: `static inline long IS_ERR(const void *ptr)'
> include/linux/err.h:31: Error: junk at end of line, first unrecognized character is `{'
> include/linux/err.h:32: Error: no such instruction: `return unlikely(((unsigned long)ptr)>=(unsigned long)-4095)'
> include/linux/err.h:33: Error: junk at end of line, first unrecognized character is `}'
> distcc[7619] ERROR: compile (null) on localhost failed
> make[1]: *** [arch/i386/kernel/vsyscall-sysenter.o] Error 1
> make: *** [arch/i386/kernel/vsyscall-sysenter.o] Error 2

unlikely() shouldn't be used in code exported to user space. At least
one architecture simply open-codes the __builtin_expect(); or we could
introduce __likely() and __unlikely() for the benefit of userspace.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Randy.Dunlap · External Since: May 15, 2006 Posts: 372

On Mon, 03 Jul 2006 08:03:29 -0700 H. Peter Anvin wrote:

> Andrew Morton wrote:
> > On Sun, 2 Jul 2006 11:27:22 -0700
> > "Randy.Dunlap" <rdunlap RemoveThis @xenotime.net> wrote:
> >
> >> --- linux-2617-g20.orig/include/asm-i386/unistd.h
> >> +++ linux-2617-g20/include/asm-i386/unistd.h
> >> @@ -327,14 +327,15 @@
> >> #ifdef __KERNEL__
> >>
> >> #define NR_syscalls 318
> >> +#include <linux/err.h>
> >
> > include/linux/err.h: Assembler messages:
> > include/linux/err.h:20: Error: no such instruction: `static inline void *ERR_PTR(long error)'
> > include/linux/err.h:21: Error: junk at end of line, first unrecognized character is `{'
> > include/linux/err.h:22: Error: no such instruction: `return (void *)error'
> > include/linux/err.h:23: Error: junk at end of line, first unrecognized character is `}'
> > include/linux/err.h:25: Error: no such instruction: `static inline long PTR_ERR(const void *ptr)'
> > include/linux/err.h:26: Error: junk at end of line, first unrecognized character is `{'
> > include/linux/err.h:27: Error: no such instruction: `return (long)ptr'
> > include/linux/err.h:28: Error: junk at end of line, first unrecognized character is `}'
> > include/linux/err.h:30: Error: no such instruction: `static inline long IS_ERR(const void *ptr)'
> > include/linux/err.h:31: Error: junk at end of line, first unrecognized character is `{'
> > include/linux/err.h:32: Error: no such instruction: `return unlikely(((unsigned long)ptr)>=(unsigned long)-4095)'
> > include/linux/err.h:33: Error: junk at end of line, first unrecognized character is `}'
> > distcc[7619] ERROR: compile (null) on localhost failed
> > make[1]: *** [arch/i386/kernel/vsyscall-sysenter.o] Error 1
> > make: *** [arch/i386/kernel/vsyscall-sysenter.o] Error 2

Built for me on i386 and x86_64.

> unlikely() shouldn't be used in code exported to user space. At least
> one architecture simply open-codes the __builtin_expect(); or we could
> introduce __likely() and __unlikely() for the benefit of userspace.

How did you determine that this had something to do with
userspace?

---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

H. Peter Anvin · External Since: Feb 08, 2006 Posts: 988

Randy.Dunlap wrote:
>>>>
>>>> #define NR_syscalls 318
>>>> +#include <linux/err.h>
>>> include/linux/err.h: Assembler messages:
>>> include/linux/err.h:20: Error: no such instruction: `static inline void *ERR_PTR(long error)'
>>> include/linux/err.h:21: Error: junk at end of line, first unrecognized character is `{'
>>> include/linux/err.h:22: Error: no such instruction: `return (void *)error'
>>> include/linux/err.h:23: Error: junk at end of line, first unrecognized character is `}'
>>> include/linux/err.h:25: Error: no such instruction: `static inline long PTR_ERR(const void *ptr)'
>>> include/linux/err.h:26: Error: junk at end of line, first unrecognized character is `{'
>>> include/linux/err.h:27: Error: no such instruction: `return (long)ptr'
>>> include/linux/err.h:28: Error: junk at end of line, first unrecognized character is `}'
>>> include/linux/err.h:30: Error: no such instruction: `static inline long IS_ERR(const void *ptr)'
>>> include/linux/err.h:31: Error: junk at end of line, first unrecognized character is `{'
>>> include/linux/err.h:32: Error: no such instruction: `return unlikely(((unsigned long)ptr)>=(unsigned long)-4095)'
>>> include/linux/err.h:33: Error: junk at end of line, first unrecognized character is `}'
>>> distcc[7619] ERROR: compile (null) on localhost failed
>>> make[1]: *** [arch/i386/kernel/vsyscall-sysenter.o] Error 1
>>> make: *** [arch/i386/kernel/vsyscall-sysenter.o] Error 2
>
> Built for me on i386 and x86_64.
>
>> unlikely() shouldn't be used in code exported to user space. At least
>> one architecture simply open-codes the __builtin_expect(); or we could
>> introduce __likely() and __unlikely() for the benefit of userspace.
>
> How did you determine that this had something to do with
> userspace?
>

The vsyscalls are userspace code. Doesn't mean they operate by
userspace naming rules, of course, but still...

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Andreas Mohr · External Since: Nov 21, 2004 Posts: 122

Hi,

On Thu, Jun 29, 2006 at 07:10:13PM +0100, Ralf Baechle wrote:
> On Wed, Jun 28, 2006 at 02:08:25PM -0700, Randy.Dunlap wrote:
>
> > Hi,
> > Peter Anvin mentioned just a few days ago that this threshold value
> > should be 4095 for all arches. I think we need to get that patch
> > done & submitted to Andrew for -mm.
>
> So here the patch is:
>
> o Raise the maximum error number in IS_ERR_VALUE to 4095.
> o Make that number available as a new constant MAX_ERRNO.
>
> Signed-off-by: Ralf Baechle <ralf.TakeThisOut@linux-mips.org>
>
> diff --git a/include/linux/err.h b/include/linux/err.h
> index ff71d2a..cd3b367 100644
> --- a/include/linux/err.h
> +++ b/include/linux/err.h
> @@ -13,7 +13,9 @@ #include <asm/errno.h>
> * This should be a per-architecture thing, to allow different
> * error and pointer decisions.
> */
> -#define IS_ERR_VALUE(x) unlikely((x) > (unsigned long)-1000L)
> +#define MAX_ERRNO 4095
> +
> +#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
>
> static inline void *ERR_PTR(long error)
> {

# cat /proc/likely_prof |grep -v "^ "

Likely Profiling Results
[+- ] Type | # True | # False | Function:Filename@Line
+unlikely | 1872| 0 IS_ERR()@:include/linux/err.h@34
-likely | 88| 1033 remove_from_swapped_list()@:mm/swap_prefetch.c@140
-likely | 311| 2679 tcp_transmit_skb()@:net/ipv4/tcp_output.c@372
-likely | 0| 175176 __switch_to_xtra()@:arch/i386/kernel/process.c@569
+unlikely | 20| 19 remap_pte_range()@:mm/memory.c@1282
+unlikely | 9| 0 signal_pending()@:include/linux/sched.h@1543
+unlikely | 9| 0 signal_pending()@:include/linux/sched.h@1543
-likely | 15591| 44490 generic_file_buffered_write()@:mm/filemap.c@2350
+unlikely | 45| 0 ll_front_merge_fn()@:block/ll_rw_blk.c@1505
+unlikely | 1| 0 simple_pin_fs()@:fs/libfs.c@419
+unlikely | 216| 0 inotify_find_update_watch()@:fs/inotify.c@597
+unlikely | 1820| 0 get_locked_pte()@:mm/memory.c@1194
+unlikely | 82529| 0 ll_back_merge_fn()@:block/ll_rw_blk.c@1467
+unlikely | 416831| 116869 need_resched()@:include/linux/sched.h@1548
+unlikely | 28334365| 23531065 __lock_acquire()@:kernel/lockdep.c@1994

# uname -a
Linux andi 2.6.19-rc1-mm1 #1 Thu Oct 19 23:55:19 CEST 2006 i686 GNU/Linux

Athlon 1200, Debian

Hmmmmmm...
(I strongly believe I've seen this particular unlikely place trigger sometime
before already!)

Could this be because

+#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)

should be made to be
#define IS_ERR_VALUE(x) unlikely((unsigned long)(x) >= (unsigned long)-MAX_ERRNO)
similar to what some other code in this discussion thread does?
(don't ask me which code is actually invoking this macro; obviously this
might be a problem due to unexpected (non-)failure which seems to be happening
here)

And some other (un)likely results here seem fishy as well, no matter
what kind of load I throw at my box... (pondering sending some patches
for the worst excesses). Any comments about those results above?

My results seem to strongly indicate that nobody else is doing unlikely
profiling??

Andreas Mohr
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Jan Engelhardt · External Since: Jun 24, 2004 Posts: 1116

>Subject: Re: IS_ERR Threshold Value
>
>> > Peter Anvin mentioned just a few days ago that this threshold value
>> > should be 4095 for all arches. I think we need to get that patch
>> > done & submitted to Andrew for -mm.
>>
>> So here the patch is:
>>
>> o Raise the maximum error number in IS_ERR_VALUE to 4095.
>>
>> +#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)

There seems to be a slight problem with doing that. Running
`ldd /bin/bash` prints out

linux-gate.so.1 => (0xffffe000)

and the topmost address a kernel function can return is 0xFFFFf000 when
MAX_ERRNO=4095, but that is going to be tight with the vdso mapped at
0xffffE000.

Or is ldd giving wrong output? Because actually, `cat /proc/$$/maps`
($$=bash) says it is mapped a lot less high:

a7fbe000-a7fbf000 r-xp a7fbe000 00:00 0 [vdso]

(CONFIG_COMPAT_VDSO=y, CONFIG_PAGE_OFFSET=0xB0000000, 2.6.1 Cool

-`J'
--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/