* Re: -mm merge plans for 2.6.21
2007-02-10 21:34 ` Ralf Baechle
@ 2007-02-11 4:53 ` Davide Libenzi
2007-02-11 15:33 ` David Woodhouse
2007-02-11 16:14 ` Heiko Carstens
2 siblings, 0 replies; 11+ messages in thread
From: Davide Libenzi @ 2007-02-11 4:53 UTC (permalink / raw)
To: Ralf Baechle
Cc: David Woodhouse, Heiko Carstens, linux-mips, Andrew Morton,
Alexey Dobriyan, Linux Kernel Mailing List, Ulrich Drepper
On Sat, 10 Feb 2007, Ralf Baechle wrote:
> Unfortunately struct epoll_event contains a gap so it bets on identical
> padding rules between native and compat ABI and anyway, padding is wasted
> space so the struct members should have been swapped when this structure
> was created. Oh well, too late.
You really should have needed padding in there, since even if you swapped
the members, sizeof(struct epoll_event) would still need to be 16, if
alignof(u64) == 8. Either adding an extra auxilliary u32, or make the two
members be u64, would have been ok.
I'll be posting a patch that adds the compat_ layer for epoll in
kernel/compat.c. Architectures that needs it (currently only ARM-EABI and
IA64 use a compat code for epoll), should wire them up.
- Davide
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: -mm merge plans for 2.6.21
2007-02-10 21:34 ` Ralf Baechle
2007-02-11 4:53 ` Davide Libenzi
@ 2007-02-11 15:33 ` David Woodhouse
2007-02-11 16:09 ` Ralf Baechle
2007-02-11 16:14 ` Heiko Carstens
2 siblings, 1 reply; 11+ messages in thread
From: David Woodhouse @ 2007-02-11 15:33 UTC (permalink / raw)
To: Ralf Baechle
Cc: Heiko Carstens, Davide Libenzi, linux-mips, Andrew Morton,
Alexey Dobriyan, Linux Kernel Mailing List, Ulrich Drepper
On Sat, 2007-02-10 at 21:34 +0000, Ralf Baechle wrote:
> Unfortunately struct epoll_event contains a gap so it bets on identical
> padding rules between native and compat ABI and anyway, padding is wasted
> space so the struct members should have been swapped when this structure
> was created. Oh well, too late.
Indeed. That was the example I was thinking of when I suggested the "no
new syscalls without _simultaneous_ compat version" rule.
--
dwmw2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: -mm merge plans for 2.6.21
2007-02-11 15:33 ` David Woodhouse
@ 2007-02-11 16:09 ` Ralf Baechle
0 siblings, 0 replies; 11+ messages in thread
From: Ralf Baechle @ 2007-02-11 16:09 UTC (permalink / raw)
To: David Woodhouse
Cc: Heiko Carstens, Davide Libenzi, linux-mips, Andrew Morton,
Alexey Dobriyan, Linux Kernel Mailing List, Ulrich Drepper
On Sun, Feb 11, 2007 at 04:33:41PM +0100, David Woodhouse wrote:
> On Sat, 2007-02-10 at 21:34 +0000, Ralf Baechle wrote:
> > Unfortunately struct epoll_event contains a gap so it bets on identical
> > padding rules between native and compat ABI and anyway, padding is wasted
> > space so the struct members should have been swapped when this structure
> > was created. Oh well, too late.
>
> Indeed. That was the example I was thinking of when I suggested the "no
> new syscalls without _simultaneous_ compat version" rule.
The need for a compat ABI is not always obvious.
Ralf
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: -mm merge plans for 2.6.21
2007-02-10 21:34 ` Ralf Baechle
2007-02-11 4:53 ` Davide Libenzi
2007-02-11 15:33 ` David Woodhouse
@ 2007-02-11 16:14 ` Heiko Carstens
2007-02-11 16:34 ` Davide Libenzi
2007-02-11 18:01 ` Ralf Baechle
2 siblings, 2 replies; 11+ messages in thread
From: Heiko Carstens @ 2007-02-11 16:14 UTC (permalink / raw)
To: Ralf Baechle
Cc: David Woodhouse, Davide Libenzi, linux-mips, Andrew Morton,
Alexey Dobriyan, Linux Kernel Mailing List, Ulrich Drepper
On Sat, Feb 10, 2007 at 09:34:47PM +0000, Ralf Baechle wrote:
> On Sat, Feb 10, 2007 at 10:32:07AM +0000, David Woodhouse wrote:
>
> > On Sat, 2007-02-10 at 11:22 +0100, Heiko Carstens wrote:
> > > Which remembers me that I think that MIPS is using the non-compat version
> > > of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
> > > syscall for some reason. Dunno.
> >
> > It's OK as long as the 64-bit kernel, N32 and O32 userspace all agree
> > there there's 32 bits of padding between the fields of this structure:
> >
> > struct epoll_event {
> > __u32 events;
> > __u64 data;
> > };
> >
> > I suspect it's a fairly safe bet that N32 userspace agrees; if the O32
> > ABI is different then it would need the compat syscall.
>
> That is correct - and apparently for all ABIs because I wasn't able to find
> a compat_sys_epoll_pwait at all.
Hmm.. so you don't need to do some fancy compat conversion for the sigset_t
that gets passed? Why is that? I don't get it...
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: -mm merge plans for 2.6.21
2007-02-11 16:14 ` Heiko Carstens
@ 2007-02-11 16:34 ` Davide Libenzi
2007-02-11 18:01 ` Ralf Baechle
1 sibling, 0 replies; 11+ messages in thread
From: Davide Libenzi @ 2007-02-11 16:34 UTC (permalink / raw)
To: Heiko Carstens
Cc: Ralf Baechle, David Woodhouse, linux-mips, Andrew Morton,
Alexey Dobriyan, Linux Kernel Mailing List, Ulrich Drepper
On Sun, 11 Feb 2007, Heiko Carstens wrote:
> On Sat, Feb 10, 2007 at 09:34:47PM +0000, Ralf Baechle wrote:
> > On Sat, Feb 10, 2007 at 10:32:07AM +0000, David Woodhouse wrote:
> >
> > > On Sat, 2007-02-10 at 11:22 +0100, Heiko Carstens wrote:
> > > > Which remembers me that I think that MIPS is using the non-compat version
> > > > of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
> > > > syscall for some reason. Dunno.
> > >
> > > It's OK as long as the 64-bit kernel, N32 and O32 userspace all agree
> > > there there's 32 bits of padding between the fields of this structure:
> > >
> > > struct epoll_event {
> > > __u32 events;
> > > __u64 data;
> > > };
> > >
> > > I suspect it's a fairly safe bet that N32 userspace agrees; if the O32
> > > ABI is different then it would need the compat syscall.
> >
> > That is correct - and apparently for all ABIs because I wasn't able to find
> > a compat_sys_epoll_pwait at all.
>
> Hmm.. so you don't need to do some fancy compat conversion for the sigset_t
> that gets passed? Why is that? I don't get it...
The compat_sys_epoll_pwait function has two sources of compat. One is the
sigset_t and the other one is the struct epoll_event. The sigset_t compat
is always needed, while the struct epoll_event may be needed. The code of
(upcoming) compat_sys_epoll_pwait takes care of both, with a smart
compile-time check to wire sys_epoll_wait or compat_sys_epoll_wait,
depending on the need of the struct epoll_event compat handling.
So yes, compat_sys_epoll_pwait is needed.
- Davide
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: -mm merge plans for 2.6.21
2007-02-11 16:14 ` Heiko Carstens
2007-02-11 16:34 ` Davide Libenzi
@ 2007-02-11 18:01 ` Ralf Baechle
1 sibling, 0 replies; 11+ messages in thread
From: Ralf Baechle @ 2007-02-11 18:01 UTC (permalink / raw)
To: Heiko Carstens
Cc: David Woodhouse, Davide Libenzi, linux-mips, Andrew Morton,
Alexey Dobriyan, Linux Kernel Mailing List, Ulrich Drepper
On Sun, Feb 11, 2007 at 05:14:46PM +0100, Heiko Carstens wrote:
> Hmm.. so you don't need to do some fancy compat conversion for the sigset_t
> that gets passed? Why is that? I don't get it...
Ah, I finally get your point. Yes, that needs conversion.
Ralf
^ permalink raw reply [flat|nested] 11+ messages in thread