epoll handling in the alpha port

epoll handling in the alpha port

[Mike Frysinger]

the alpha linux port differs from all others when it comes to the epoll 
functions in that it uses '__NR_sys_epoll_XXX' instead of '__NR_epoll_XXX' in 
the asm/unistd.h header ... the trouble with this is that glibc maps the 
function name directly to an __NR_ define, so when it maps 
like 'epoll_create', it expects '__NR_epoll_create' (which it gets with all 
other architectures)

is there some obvious thing i'm missing here ?  or is the attached patch OK ?

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
--- a/include/asm-alpha/unistd.h
+++ b/include/asm-alpha/unistd.h
@@ -342,9 +342,9 @@
 #define __NR_io_cancel			402
 #define __NR_exit_group			405
 #define __NR_lookup_dcookie		406
-#define __NR_sys_epoll_create		407
-#define __NR_sys_epoll_ctl		408
-#define __NR_sys_epoll_wait		409
+#define __NR_epoll_create		407
+#define __NR_epoll_ctl		408
+#define __NR_epoll_wait		409
 #define __NR_remap_file_pages		410
 #define __NR_set_tid_address		411
 #define __NR_restart_syscall		412

Re: epoll handling in the alpha port

[Andrew Morton]

On Thu, 1 Feb 2007 15:20:45 -0500 Mike Frysinger <vapier@gentoo.org> wrote:

> the alpha linux port differs from all others when it comes to the epoll 
> functions in that it uses '__NR_sys_epoll_XXX' instead of '__NR_epoll_XXX' in 
> the asm/unistd.h header ... the trouble with this is that glibc maps the 
> function name directly to an __NR_ define, so when it maps 
> like 'epoll_create', it expects '__NR_epoll_create' (which it gets with all 
> other architectures)
> 
> is there some obvious thing i'm missing here ?  or is the attached patch OK ?
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> --- a/include/asm-alpha/unistd.h
> +++ b/include/asm-alpha/unistd.h
> @@ -342,9 +342,9 @@
>  #define __NR_io_cancel			402
>  #define __NR_exit_group			405
>  #define __NR_lookup_dcookie		406
> -#define __NR_sys_epoll_create		407
> -#define __NR_sys_epoll_ctl		408
> -#define __NR_sys_epoll_wait		409
> +#define __NR_epoll_create		407
> +#define __NR_epoll_ctl		408
> +#define __NR_epoll_wait		409
>  #define __NR_remap_file_pages		410
>  #define __NR_set_tid_address		411
>  #define __NR_restart_syscall		412

It might be to late to fix this - we risk breaking userspace which worked
around it.

It would be safer to add the correct definitions while leaving the old ones
in place.

Re: epoll handling in the alpha port

[Mike Frysinger]

On Friday 02 February 2007, Andrew Morton wrote:
> It might be to late to fix this - we risk breaking userspace which worked
> around it.

we'd be breaking userspace API, but not ABI ... and the largest consumers out 
there (glibc/uclibc) have not been accounting for this, so we wouldnt be 
breaking them

that's how we noticed the issue in Gentoo ... all the epoll calls are 
automatically declared -ENOSYS stubs in glibc's libc.so

is there some other large consumer i'm not thinking of ?

> It would be safer to add the correct definitions while leaving the old ones
> in place.

in the end, either works for me ... i can see linux's pretty hard 'dont break 
userspace' policy coming into play here
-mike

Re: epoll handling in the alpha port

[Willy Tarreau]

On Fri, Feb 02, 2007 at 12:19:35AM -0500, Mike Frysinger wrote:
> On Friday 02 February 2007, Andrew Morton wrote:
> > It might be to late to fix this - we risk breaking userspace which worked
> > around it.
> 
> we'd be breaking userspace API, but not ABI ... and the largest consumers out 
> there (glibc/uclibc) have not been accounting for this, so we wouldnt be 
> breaking them
> 
> that's how we noticed the issue in Gentoo ... all the epoll calls are 
> automatically declared -ENOSYS stubs in glibc's libc.so
> 
> is there some other large consumer i'm not thinking of ?

Not necessarily some large consumers, but epoll() is quite older than its
kernel inclusion, so there certainly are some apps still using the syscall
without going through libc.

> > It would be safer to add the correct definitions while leaving the old ones
> > in place.
> 
> in the end, either works for me ... i can see linux's pretty hard 'dont break 
> userspace' policy coming into play here

I think it would be reasonable to use the NR_epoll* names first, and to
declare NR_sys_epoll* aliases which will point to NR_epoll*. You would
then add a comment with the date of the change, stating that you keep
them just in case there are some old apps. Then, someone coming on the
comment in 3 or 4 years could reasonably suggest removing those aliases.

Willy

Re: epoll handling in the alpha port

[Mike Frysinger]

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

On Friday 02 February 2007, Willy Tarreau wrote:
> I think it would be reasonable to use the NR_epoll* names first, and to
> declare NR_sys_epoll* aliases which will point to NR_epoll*. You would
> then add a comment with the date of the change, stating that you keep
> them just in case there are some old apps. Then, someone coming on the
> comment in 3 or 4 years could reasonably suggest removing those aliases.

fair enough, sounds like a plan man ... patch attached to do that

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/include/asm-alpha/unistd.h b/include/asm-alpha/unistd.h
index 84313d1..e58a427 100644
--- a/include/asm-alpha/unistd.h
+++ b/include/asm-alpha/unistd.h
@@ -342,9 +342,14 @@
 #define __NR_io_cancel			402
 #define __NR_exit_group			405
 #define __NR_lookup_dcookie		406
-#define __NR_sys_epoll_create		407
-#define __NR_sys_epoll_ctl		408
-#define __NR_sys_epoll_wait		409
+#define __NR_epoll_create		407
+#define __NR_epoll_ctl			408
+#define __NR_epoll_wait			409
+/* Feb 2007: These three sys_epoll defines shouldn't be here but culling
+ * them would break userspace apps ... we'll kill them off in 2010 :) */
+#define __NR_sys_epoll_create		__NR_epoll_create
+#define __NR_sys_epoll_ctl		__NR_epoll_ctl
+#define __NR_sys_epoll_wait		__NR_epoll_wait
 #define __NR_remap_file_pages		410
 #define __NR_set_tid_address		411
 #define __NR_restart_syscall		412

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]