opaque types instead of union epoll_data

opaque types instead of union epoll_data

[Sodagudi Prasad]

Hi All,

uapi structs epoll_data are more opaque than user space expects. kernel 
have defined as __u64 instead of the union epoll_data.
Because of this libc have redefined struct epoll_event with union data 
member.

https://android.googlesource.com/platform/bionic.git/+/master/libc/include/sys/epoll.h
typedef union epoll_data {
   void* ptr;
   int fd;
   uint32_t u32;
   uint64_t u64;
} epoll_data_t;
struct epoll_event {
   uint32_t events;
   epoll_data_t data;
}

Kernel UAPI header defined as "include/uapi/linux/eventpoll.h"
struct epoll_event {
         __u32 events;
         __u64 data;  =====>opaque type instead of union epoll_data
} EPOLL_PACKED;


Because of this we are landing into some issues as we copy kernel 
headers. Will it be going to be addressed?

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
Linux Foundation Collaborative Project

Re: opaque types instead of union epoll_data

[Carlos O'Donell]

On 03/07/2017 07:31 AM, Sodagudi Prasad wrote:
> uapi structs epoll_data are more opaque than user space expects.
> kernel have defined as __u64 instead of the union epoll_data. Because
> of this libc have redefined struct epoll_event with union data
> member.

We do the same in glibc.

> https://android.googlesource.com/platform/bionic.git/+/master/libc/include/sys/epoll.h
> typedef union epoll_data {
>   void* ptr;
>   int fd;
>   uint32_t u32;
>   uint64_t u64;
> } epoll_data_t;
> struct epoll_event {
>   uint32_t events;
>   epoll_data_t data;
> }
> 
> Kernel UAPI header defined as "include/uapi/linux/eventpoll.h"
> struct epoll_event {
>         __u32 events;
>         __u64 data;  =====>opaque type instead of union epoll_data
> } EPOLL_PACKED;
> 
> 
> Because of this we are landing into some issues as we copy kernel
> headers. Will it be going to be addressed?

What issues are you having? 

Exactly what problems are you running in to? 

Using all of the UAPI headers directly is not a workable
solution, there is a lot of definition and cleanup work to do there.
Some of the headers are immediately useful, but others, as you note
require quite a bit of work to become useful.

The underlying issue of a mismatch between userspace expectations
and UAPI definitions will get addressed when someone, maybe you, 
works diligently to enhance the kernel UAPI headers to be generally 
useful to userspace.

I don't know anyone else who is working on this problem. Though I
have a vested interested in it as a glibc maintainer, since it would
be nice to avoid duplicate headers where possible between the kernel
and userspace.

-- 
Cheers,
Carlos.

Re: opaque types instead of union epoll_data

[Greg KH]

On Tue, Mar 07, 2017 at 09:33:57AM -0500, Carlos O'Donell wrote:
> On 03/07/2017 07:31 AM, Sodagudi Prasad wrote:
> > uapi structs epoll_data are more opaque than user space expects.
> > kernel have defined as __u64 instead of the union epoll_data. Because
> > of this libc have redefined struct epoll_event with union data
> > member.
> 
> We do the same in glibc.
> 
> > https://android.googlesource.com/platform/bionic.git/+/master/libc/include/sys/epoll.h
> > typedef union epoll_data {
> >   void* ptr;
> >   int fd;
> >   uint32_t u32;
> >   uint64_t u64;
> > } epoll_data_t;
> > struct epoll_event {
> >   uint32_t events;
> >   epoll_data_t data;
> > }
> > 
> > Kernel UAPI header defined as "include/uapi/linux/eventpoll.h"
> > struct epoll_event {
> >         __u32 events;
> >         __u64 data;  =====>opaque type instead of union epoll_data
> > } EPOLL_PACKED;
> > 
> > 
> > Because of this we are landing into some issues as we copy kernel
> > headers. Will it be going to be addressed?
> 
> What issues are you having? 
> 
> Exactly what problems are you running in to? 
> 
> Using all of the UAPI headers directly is not a workable
> solution, there is a lot of definition and cleanup work to do there.
> Some of the headers are immediately useful, but others, as you note
> require quite a bit of work to become useful.
> 
> The underlying issue of a mismatch between userspace expectations
> and UAPI definitions will get addressed when someone, maybe you, 
> works diligently to enhance the kernel UAPI headers to be generally 
> useful to userspace.
> 
> I don't know anyone else who is working on this problem. Though I
> have a vested interested in it as a glibc maintainer, since it would
> be nice to avoid duplicate headers where possible between the kernel
> and userspace.

I've been working with the bionic developers to try to help clean up
some of these things.  Yes, epoll is messy here as you all have pointed
out, and I'll be glad to review any patches that people submit for this.
I need to get some spare time (hah!) to work through some of the issues
that the bionic developers have already pointed out to me...

thanks,

greg k-h

Re: opaque types instead of union epoll_data

[Carlos O'Donell]

On 03/07/2017 12:59 PM, Greg KH wrote:
> On Tue, Mar 07, 2017 at 09:33:57AM -0500, Carlos O'Donell wrote:
>> I don't know anyone else who is working on this problem. Though I
>> have a vested interested in it as a glibc maintainer, since it would
>> be nice to avoid duplicate headers where possible between the kernel
>> and userspace.
> 
> I've been working with the bionic developers to try to help clean up
> some of these things.  Yes, epoll is messy here as you all have pointed
> out, and I'll be glad to review any patches that people submit for this.
> I need to get some spare time (hah!) to work through some of the issues
> that the bionic developers have already pointed out to me...

Sounds awesome. Please keep linux-api in the loop so the other libc
maintainers can follow along. From a glibc perspective I'm interested,
but also busy (see the other thread about libc-compat.h which has
immediate impact on header coordination).

-- 
Cheers,
Carlos.

Re: opaque types instead of union epoll_data

[Greg KH]

On Wed, Mar 08, 2017 at 12:34:07PM -0500, Carlos O'Donell wrote:
> On 03/07/2017 12:59 PM, Greg KH wrote:
> > On Tue, Mar 07, 2017 at 09:33:57AM -0500, Carlos O'Donell wrote:
> >> I don't know anyone else who is working on this problem. Though I
> >> have a vested interested in it as a glibc maintainer, since it would
> >> be nice to avoid duplicate headers where possible between the kernel
> >> and userspace.
> > 
> > I've been working with the bionic developers to try to help clean up
> > some of these things.  Yes, epoll is messy here as you all have pointed
> > out, and I'll be glad to review any patches that people submit for this.
> > I need to get some spare time (hah!) to work through some of the issues
> > that the bionic developers have already pointed out to me...
> 
> Sounds awesome. Please keep linux-api in the loop so the other libc
> maintainers can follow along. From a glibc perspective I'm interested,
> but also busy (see the other thread about libc-compat.h which has
> immediate impact on header coordination).

Ah, I forgot to cc: linux-api on my last epoll header file change, I'll
go do that now before I commit it to my tree for 4.12...

thanks,

greg k-h