Re: [take12 0/3] kevent: Generic event handling mechanism.

27 messages, 8 authors, 2006-08-23 · open the first message on its own page

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-22 18:02:40

On Tue, Aug 22, 2006 at 06:57:05PM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
On 8/22/06, Nicholas Miell [off-list ref] wrote:
quoted

OK, so with literally a dozen different interfaces to queue events to
userspace, all of which are apparently inadequate and in need of
replacement by kevent, don't you want to slow down a bit and make sure
that the kevent API is correct before it becomes permanent and then just
has to be replaced *again* ?
Not to mention the name used causes (at least me) some confusion with BSD's
kqueue implementation. Skimming over the patches it actually looks somewhat
like kqueue with the more interesting features removed, like the ability to
pass the filter changes simultaneously with polling.
I do not understand, what do you mean?
It is obviously allowed to poll and change kevents at the same time.
Maybe this is a topic that will singe my fur, but what is wrong with the
kqueue API? Will I really have to implement support for yet another event
API in my program.
Why did I not implemented it like Solaris did?
Or FreeBSD did?
It was designed with features mention on AIO homepage in mind, but not
to be compatible with some other implementation.
And why should it be?
Rakshasa
-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Jari Sundell <hidden>
Date: 2006-08-22 19:14:33

On 8/22/06, Evgeniy Polyakov [off-list ref] wrote:
quoted
Not to mention the name used causes (at least me) some confusion with BSD's
kqueue implementation. Skimming over the patches it actually looks somewhat
like kqueue with the more interesting features removed, like the ability to
pass the filter changes simultaneously with polling.
I do not understand, what do you mean?
It is obviously allowed to poll and change kevents at the same time.
Changing kevents are done with a separate system call from polling
afaics, thus every change requires a context switch. This in contrast
to BSD's kqueue which allows user-space to pass the changes when
kevent (polling) is called.

It may also choose to update the filters immediately with the same call.
quoted
Maybe this is a topic that will singe my fur, but what is wrong with the
kqueue API? Will I really have to implement support for yet another event
API in my program.
Why did I not implemented it like Solaris did?
Or FreeBSD did?
It was designed with features mention on AIO homepage in mind, but not
to be compatible with some other implementation.
And why should it be?
If it can be, why should it not be? At least, if you reinvent the
wheel its advantages should be obvious.

Considering that kqueue is available on more popular OSes like darwin
it would ease portability greatly if there was a shared event API.
That is, unless you think there's something fundamentally wrong with
their design.

Your interface:

+asmlinkage long sys_kevent_get_events(int ctl_fd, unsigned int min,
unsigned int max,
+               unsigned int timeout, void __user *buf, unsigned flags);
+asmlinkage long sys_kevent_ctl(int ctl_fd, unsigned int cmd, unsigned
int num, void __user *buf);

BSD's kqueue:

struct kevent {
  uintptr_t ident;        /* identifier for this event */
  short     filter;       /* filter for event */
  u_short   flags;        /* action flags for kqueue */
  u_int     fflags;       /* filter flag value */
  intptr_t  data;         /* filter data value */
  void      *udata;       /* opaque user data identifier */
};

int kevent(int kq, const struct kevent *changelist, int nchanges,
struct kevent *eventlist, int nevents, const struct timespec
*timeout);

The only thing missing in BSD's kevent is the min/max parameters, the
various filters in kevent_get_events either have equivalent filters or
could be added as extensions. (I didn't look too carefully through
them)

On the other hand, your API lacks the ability to pass changes when
polling, as mentioned above. It would be preferable if the timeout
parameter was either timespec or timeval.

Rakshasa

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-22 19:48:36

On Tue, Aug 22, 2006 at 09:14:30PM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
Changing kevents are done with a separate system call from polling
afaics, thus every change requires a context switch. This in contrast
to BSD's kqueue which allows user-space to pass the changes when
kevent (polling) is called.

It may also choose to update the filters immediately with the same call.
Word "polling" really confuses me here, but now I understand you.
Such approach actually has unresolved issues - consider for
example a situation when all provided events are ready immediately - what
should be returned (as far as I recall they are always added into kqueue in
BSDs before started to be checked, so old events will be returned
first)? And currently ready events can be read through mapped buffer
without any syscall at all.
And Linux syscall is much cheaper than BSD's one.
Consider (especially apped buffer)  that issues, it really does not cost
interface complexity.
quoted
quoted
Maybe this is a topic that will singe my fur, but what is wrong with the
kqueue API? Will I really have to implement support for yet another event
API in my program.
Why did I not implemented it like Solaris did?
Or FreeBSD did?
It was designed with features mention on AIO homepage in mind, but not
to be compatible with some other implementation.
And why should it be?
If it can be, why should it not be? At least, if you reinvent the
wheel its advantages should be obvious.

Considering that kqueue is available on more popular OSes like darwin
it would ease portability greatly if there was a shared event API.
That is, unless you think there's something fundamentally wrong with
their design.
First of all, there are completely different types.
Design of the in-kernel part is very different too.
Your interface:

+asmlinkage long sys_kevent_get_events(int ctl_fd, unsigned int min,
unsigned int max,
+               unsigned int timeout, void __user *buf, unsigned flags);
+asmlinkage long sys_kevent_ctl(int ctl_fd, unsigned int cmd, unsigned
int num, void __user *buf);

BSD's kqueue:

struct kevent {
 uintptr_t ident;        /* identifier for this event */
 short     filter;       /* filter for event */
 u_short   flags;        /* action flags for kqueue */
 u_int     fflags;       /* filter flag value */
 intptr_t  data;         /* filter data value */
 void      *udata;       /* opaque user data identifier */
};
From your description there is a serious problem with arches which
supports different width of the pointer. I do not have sources of ny BSD
right now, but if it is really like you've described, it can not be used
in Linux at all.
int kevent(int kq, const struct kevent *changelist, int nchanges,
struct kevent *eventlist, int nevents, const struct timespec
*timeout);

The only thing missing in BSD's kevent is the min/max parameters, the
various filters in kevent_get_events either have equivalent filters or
could be added as extensions. (I didn't look too carefully through
them)

On the other hand, your API lacks the ability to pass changes when
polling, as mentioned above. It would be preferable if the timeout
parameter was either timespec or timeval.
No way - timespec uses long.
Rakshasa
-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Jari Sundell <hidden>
Date: 2006-08-22 22:51:13

On 8/22/06, Evgeniy Polyakov [off-list ref] wrote:
Word "polling" really confuses me here, but now I understand you.
Such approach actually has unresolved issues - consider for
example a situation when all provided events are ready immediately - what
should be returned (as far as I recall they are always added into kqueue in
BSDs before started to be checked, so old events will be returned
first)? And currently ready events can be read through mapped buffer
without any syscall at all.
And Linux syscall is much cheaper than BSD's one.
Consider (especially apped buffer)  that issues, it really does not cost
interface complexity.
There's no reason I can see that kqueue's kevent should not be able to
check an mmaped buffer as in your implementation, after having passed
any filter changes to the kernel.

I'm not sure if I read you correctly, but the situation where all
events are ready immediately is not a problem. Only the delta is
passed with the kevent call, so old events will still be first in the
queue. And as long as the user doesn't randomize the order of the
changelist and passes the changedlist with each kevent call, the
resulting order in which changes are received will be no different
from using individual system calls.

If there's some very specific reason the user needs to retain the
order in which events happen in the interval between adding it to the
changelist and calling kevent, he may decide to call kevent
immediately without asking for any events.
First of all, there are completely different types.
Design of the in-kernel part is very different too.
The question I'm asking is not whet ever kqueue can fit this
implementation, but rather if it is possible to make the
implementation fit kqueue. I can't really see any fundemental
differences, merely implementation details. Maybe I'm just unfamiliar
with the requirements.
quoted
BSD's kqueue:

struct kevent {
 uintptr_t ident;        /* identifier for this event */
 short     filter;       /* filter for event */
 u_short   flags;        /* action flags for kqueue */
 u_int     fflags;       /* filter flag value */
 intptr_t  data;         /* filter data value */
 void      *udata;       /* opaque user data identifier */
};

From your description there is a serious problem with arches which
supports different width of the pointer. I do not have sources of ny BSD
right now, but if it is really like you've described, it can not be used
in Linux at all.
Are you referring to udata or data? I'll assume the latter as the
former is more of a restriction on user-space. intptr_t is required to
be safely convertible to a void*, so I don't see what the problem
would be.
No way - timespec uses long.
I must have missed that discussion. Please enlighten me in what regard
using an opaque type with lower resolution is preferable to a type
defined in POSIX for this sort of purpose. Considering the extra code
I need to write to properly handle having just ms resolution, it
better be something fundamentally broken. ;)

Rakshasa

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Alexey Kuznetsov <hidden>
Date: 2006-08-22 23:12:32

Hello!
quoted
No way - timespec uses long.
I must have missed that discussion. Please enlighten me in what regard
using an opaque type with lower resolution is preferable to a type
defined in POSIX for this sort of purpose.
Let me explain, as a person who did this mistake and deeply
regrets about this.

F.e. in this case you just cannot use kevents in 32bit application
on x86_64, unless you add the whole translation layer inside kevent core.
Even when you deal with plain syscall, translation is a big pain,
but when you use mmapped buffer, it can be simply impossible.

F.e. my mistake was "unsigned long" in struct tpacket_hdr in linux/if_packet.h.
It makes use of mmapped packet socket essentially impossible by 32bit
applications on 64bit archs.

Alexey

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Jari Sundell <hidden>
Date: 2006-08-23 00:28:34

On 8/23/06, Alexey Kuznetsov [off-list ref] wrote:
Let me explain, as a person who did this mistake and deeply
regrets about this.

F.e. in this case you just cannot use kevents in 32bit application
on x86_64, unless you add the whole translation layer inside kevent core.
Even when you deal with plain syscall, translation is a big pain,
but when you use mmapped buffer, it can be simply impossible.

F.e. my mistake was "unsigned long" in struct tpacket_hdr in linux/if_packet.h.
It makes use of mmapped packet socket essentially impossible by 32bit
applications on 64bit archs.
There are system calls that take timespec, so I assume the magic is
already available for handling the timeout argument of kevent.
Although I'm not entirely sure about the kqueue timer interface, there
isn't any reason timespec would need to be written to the mmaped
buffer for the rest.

AFAICS, only struct ukevent is visible to the user, same would go for
kqueue's struct kevent.

Rakshasa

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: David Miller <davem@davemloft.net>
Date: 2006-08-23 00:32:05

From: "Jari Sundell" <redacted>
Date: Wed, 23 Aug 2006 02:28:32 +0200
There are system calls that take timespec, so I assume the magic is
already available for handling the timeout argument of kevent.
System calls are one thing, they can be translated for these
kinds of situations.  But this doesn't help, and nothing at
all can be done, for datastructures exposed to userspace via
mmap()'d buffers, which is what kevent will be doing.

This is what Alexey is trying to explain to you.

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Jari Sundell <hidden>
Date: 2006-08-23 00:43:52

On 8/23/06, David Miller [off-list ref] wrote:
quoted
There are system calls that take timespec, so I assume the magic is
already available for handling the timeout argument of kevent.
System calls are one thing, they can be translated for these
kinds of situations.  But this doesn't help, and nothing at
all can be done, for datastructures exposed to userspace via
mmap()'d buffers, which is what kevent will be doing.

This is what Alexey is trying to explain to you.
Actually, I didn't miss that, it is an orthogonal issue. A timespec
timeout parameter for the syscall does not imply the use of timespec
in any timer event, etc. Nor is there any timespec timer in kqueue's
struct kevent, which is the only (interface related) thing that will
be exposed.

Rakshasa

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 06:58:19

On Wed, Aug 23, 2006 at 02:43:50AM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
Actually, I didn't miss that, it is an orthogonal issue. A timespec
timeout parameter for the syscall does not imply the use of timespec
in any timer event, etc. Nor is there any timespec timer in kqueue's
struct kevent, which is the only (interface related) thing that will
be exposed.
void * in structure exported to userspace is forbidden.
long in syscall requires wrapper in per-arch code (although that
workaround _is_ there, it does not mean that broken interface should 
be used).
poll uses millisecods - it is perfectly ok.
Rakshasa
-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Andrew Morton <hidden>
Date: 2006-08-23 07:09:09

On Wed, 23 Aug 2006 10:56:59 +0400
Evgeniy Polyakov [off-list ref] wrote:
On Wed, Aug 23, 2006 at 02:43:50AM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
quoted
Actually, I didn't miss that, it is an orthogonal issue. A timespec
timeout parameter for the syscall does not imply the use of timespec
in any timer event, etc. Nor is there any timespec timer in kqueue's
struct kevent, which is the only (interface related) thing that will
be exposed.
void * in structure exported to userspace is forbidden.
long in syscall requires wrapper in per-arch code (although that
workaround _is_ there, it does not mean that broken interface should 
be used).
poll uses millisecods - it is perfectly ok.
I wonder whether designing-in a millisecond granularity is the right thing
to do.  If in a few years the kernel is running tickless with high-res clock
interrupt sources, that might look a bit lumpy.

Switching it to a __u64 nanosecond counter would be basically free on
64-bit machines, and not very expensive on 32-bit, no?

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 07:11:23

On Wed, Aug 23, 2006 at 12:07:58AM -0700, Andrew Morton (akpm@osdl.org) wrote:
On Wed, 23 Aug 2006 10:56:59 +0400
Evgeniy Polyakov [off-list ref] wrote:
quoted
On Wed, Aug 23, 2006 at 02:43:50AM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
quoted
Actually, I didn't miss that, it is an orthogonal issue. A timespec
timeout parameter for the syscall does not imply the use of timespec
in any timer event, etc. Nor is there any timespec timer in kqueue's
struct kevent, which is the only (interface related) thing that will
be exposed.
void * in structure exported to userspace is forbidden.
long in syscall requires wrapper in per-arch code (although that
workaround _is_ there, it does not mean that broken interface should 
be used).
poll uses millisecods - it is perfectly ok.
I wonder whether designing-in a millisecond granularity is the right thing
to do.  If in a few years the kernel is running tickless with high-res clock
interrupt sources, that might look a bit lumpy.

Switching it to a __u64 nanosecond counter would be basically free on
64-bit machines, and not very expensive on 32-bit, no?
Let's then place there a structure with 64bit seconds and nanoseconds,
similar to timspec, but without longs there.
What do you think?

-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: David Miller <davem@davemloft.net>
Date: 2006-08-23 07:35:21

From: Andrew Morton <redacted>
Date: Wed, 23 Aug 2006 00:07:58 -0700
I wonder whether designing-in a millisecond granularity is the right thing
to do.  If in a few years the kernel is running tickless with high-res clock
interrupt sources, that might look a bit lumpy.

Switching it to a __u64 nanosecond counter would be basically free on
64-bit machines, and not very expensive on 32-bit, no?
If it ends up in a structure we'll need to use the "aligned_u64" type
in order to avoid problems with 32-bit x86 binaries running on 64-bit
kernels.

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Ian McDonald <hidden>
Date: 2006-08-23 07:43:54

I wonder whether designing-in a millisecond granularity is the right thing
to do.  If in a few years the kernel is running tickless with high-res clock
interrupt sources, that might look a bit lumpy.
I'd second that - when working on DCCP I've done a lot of the work in
microseconds and it made quite a difference instead of milliseconds
because of it's design.

I haven't followed kevents in great detail but it sounds like
something that could be useful for me with higher resolution timers
than milliseconds.
-- 
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 07:52:12

On Wed, Aug 23, 2006 at 12:07:58AM -0700, Andrew Morton (akpm@osdl.org) wrote:
On Wed, 23 Aug 2006 10:56:59 +0400
Evgeniy Polyakov [off-list ref] wrote:
quoted
On Wed, Aug 23, 2006 at 02:43:50AM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
quoted
Actually, I didn't miss that, it is an orthogonal issue. A timespec
timeout parameter for the syscall does not imply the use of timespec
in any timer event, etc. Nor is there any timespec timer in kqueue's
struct kevent, which is the only (interface related) thing that will
be exposed.
void * in structure exported to userspace is forbidden.
long in syscall requires wrapper in per-arch code (although that
workaround _is_ there, it does not mean that broken interface should 
be used).
poll uses millisecods - it is perfectly ok.
I wonder whether designing-in a millisecond granularity is the right thing
to do.  If in a few years the kernel is running tickless with high-res clock
interrupt sources, that might look a bit lumpy.

Switching it to a __u64 nanosecond counter would be basically free on
64-bit machines, and not very expensive on 32-bit, no?
I can put nanoseconds as timer interval too (with aligned_u64 as David
mentioned), and put it for timeout value too - 64 bit nanosecods ends up
with 58 years, probably enough.
Structures with u64 a really not so good idea.

-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Nicholas Miell <hidden>
Date: 2006-08-23 08:19:37

On Wed, 2006-08-23 at 00:35 -0700, David Miller wrote:
From: Andrew Morton <redacted>
Date: Wed, 23 Aug 2006 00:07:58 -0700
quoted
I wonder whether designing-in a millisecond granularity is the right thing
to do.  If in a few years the kernel is running tickless with high-res clock
interrupt sources, that might look a bit lumpy.

Switching it to a __u64 nanosecond counter would be basically free on
64-bit machines, and not very expensive on 32-bit, no?
If it ends up in a structure we'll need to use the "aligned_u64" type
in order to avoid problems with 32-bit x86 binaries running on 64-bit
kernels.
Perhaps

struct timespec64
{
	uint64_t tv_sec __attribute__((aligned(8)));
	uint32_t tv_nsec;
}

with a snide remark about gcc in the comments?

-- 
Nicholas Miell [off-list ref]

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Jari Sundell <hidden>
Date: 2006-08-23 08:22:19

On 8/23/06, Evgeniy Polyakov [off-list ref] wrote:
void * in structure exported to userspace is forbidden.
Only void * I'm seeing belongs to the user, (udata) perhaps you are
talking of something different?
long in syscall requires wrapper in per-arch code (although that
workaround _is_ there, it does not mean that broken interface should
be used).
poll uses millisecods - it is perfectly ok.
The kernel is there to hide those ugly implementation details from the
user, so I don't care that much about a workaround being required in
some cases. More important, IMHO is consistency with the POSIX system
calls.

I guess as long as you use usec, at least it won't be a pain to use.

Rakshasa

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 08:45:07

On Wed, Aug 23, 2006 at 10:22:06AM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
On 8/23/06, Evgeniy Polyakov [off-list ref] wrote:
quoted
void * in structure exported to userspace is forbidden.
Only void * I'm seeing belongs to the user, (udata) perhaps you are
talking of something different?
Yes, exactly about it.

I put union {
	u32 a[2];
	void *b;
} 
epcially to eliminate that problem.

And I'm not that sure aboit stuff like uptr_t or how they call pointers
in userspace and kernelspace.
quoted
long in syscall requires wrapper in per-arch code (although that
workaround _is_ there, it does not mean that broken interface should
be used).
poll uses millisecods - it is perfectly ok.
The kernel is there to hide those ugly implementation details from the
user, so I don't care that much about a workaround being required in
some cases. More important, IMHO is consistency with the POSIX system
calls.

I guess as long as you use usec, at least it won't be a pain to use.
Andrew suggested to use nanoseconds there in u64 variable.
I think it is ok.
Rakshasa
-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Jari Sundell <hidden>
Date: 2006-08-23 09:49:25

On 8/23/06, Evgeniy Polyakov [off-list ref] wrote:
On Wed, Aug 23, 2006 at 10:22:06AM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
quoted
On 8/23/06, Evgeniy Polyakov [off-list ref] wrote:
quoted
void * in structure exported to userspace is forbidden.
Only void * I'm seeing belongs to the user, (udata) perhaps you are
talking of something different?
Yes, exactly about it.

I put union {
        u32 a[2];
        void *b;
}
epcially to eliminate that problem.
It's just random data of a known maximum size appended to the struct,
I'm sure you can find a clean way to handle it. If you mangle the
first variable name in your union, you'll end up with something that
should be usable instead of udata.
And I'm not that sure aboit stuff like uptr_t or how they call pointers
in userspace and kernelspace.
Well, I can't find any use of pointers in your struct ukevent, nor in
any of the kqueue events in my man page. So if this is a deficit it
applies to both, I guess?
ukevent is aligned to 8 bytes already (it's size selected to be 40 bytes),
so it should not be a problem.
quoted
Eric
Even if it is so, wouldn't it be better to be explicit about it?

Rakshasa

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Andi Kleen <hidden>
Date: 2006-08-23 09:58:32

Evgeniy Polyakov [off-list ref] writes:
Let's then place there a structure with 64bit seconds and nanoseconds,
similar to timspec, but without longs there.
You need 64bit (or at least more than 32bit) for the seconds,
otherwise you add a y2038 problem which would be sad in new code.
Remember you might be still alive then ;-)

Ok one could argue that on 32bit architectures 2038 is so deeply
embedded that it doesn't make much difference, but I still
think it would be better to not readd it to new interfaces there.

64bit longs on 32bit is fine, as long as you use aligned_u64,
never long long or u64 (which has varying alignment between i386 and x86-64)

-Andi

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 10:05:23

On Wed, Aug 23, 2006 at 11:58:20AM +0200, Andi Kleen (ak@suse.de) wrote:
Evgeniy Polyakov [off-list ref] writes:
quoted
Let's then place there a structure with 64bit seconds and nanoseconds,
similar to timspec, but without longs there.
You need 64bit (or at least more than 32bit) for the seconds,
otherwise you add a y2038 problem which would be sad in new code.
Remember you might be still alive then ;-)
I hope so :)
Ok one could argue that on 32bit architectures 2038 is so deeply
embedded that it doesn't make much difference, but I still
think it would be better to not readd it to new interfaces there.

64bit longs on 32bit is fine, as long as you use aligned_u64,
never long long or u64 (which has varying alignment between i386 and x86-64)
Btw, aligned_u64 is not exported to userspace.
I commited a change with __u64 nanoseconds without any strucutres.
Do we really need a structure?
-Andi
-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 10:21:49

On Wed, Aug 23, 2006 at 11:49:22AM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
quoted
quoted
Only void * I'm seeing belongs to the user, (udata) perhaps you are
talking of something different?
Yes, exactly about it.

I put union {
       u32 a[2];
       void *b;
}
epcially to eliminate that problem.
It's just random data of a known maximum size appended to the struct,
I'm sure you can find a clean way to handle it. If you mangle the
first variable name in your union, you'll end up with something that
should be usable instead of udata.
If there will be usual pointer, size of the whole structure will be
different in kernel and userspace.
quoted
And I'm not that sure aboit stuff like uptr_t or how they call pointers
in userspace and kernelspace.
Well, I can't find any use of pointers in your struct ukevent, nor in
any of the kqueue events in my man page. So if this is a deficit it
applies to both, I guess?
No, it will change sizes of the structure in kernelspace and userspace,
so they just can not communicate.
quoted
ukevent is aligned to 8 bytes already (it's size selected to be 40 bytes),
so it should not be a problem.
quoted
Eric
Even if it is so, wouldn't it be better to be explicit about it?
Ok, I will add a comment about it.
Rakshasa
-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Jari Sundell <hidden>
Date: 2006-08-23 10:34:28

On 8/23/06, Evgeniy Polyakov [off-list ref] wrote:
No, it will change sizes of the structure in kernelspace and userspace,
so they just can not communicate.
struct kevent {
  uintptr_t ident;        /* identifier for this event */
  short     filter;       /* filter for event */
  u_short   flags;        /* action flags for kqueue */
  u_int     fflags;       /* filter flag value */

  union {
    u32       _data_padding[2];
    intptr_t  data;         /* filter data value */
  };

  union {
    u32       _udata_padding[2];
    void      *udata;       /* opaque user data identifier */
  };
};

I'm not missing anything obvious here, I hope.

Rakshasa

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 10:53:19

On Wed, Aug 23, 2006 at 12:34:25PM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
On 8/23/06, Evgeniy Polyakov [off-list ref] wrote:
quoted
No, it will change sizes of the structure in kernelspace and userspace,
so they just can not communicate.
struct kevent {
 uintptr_t ident;        /* identifier for this event */
 short     filter;       /* filter for event */
 u_short   flags;        /* action flags for kqueue */
 u_int     fflags;       /* filter flag value */

 union {
   u32       _data_padding[2];
   intptr_t  data;         /* filter data value */
 };
As Eric pointed it must be aligned.
 union {
   u32       _udata_padding[2];
   void      *udata;       /* opaque user data identifier */
 };
};

I'm not missing anything obvious here, I hope.
We still do not know what uintptr_t is, and it looks like it is a pointer, 
which is forbidden. Those numbers are not enough to make network AIO.
And actually is not compatible with kqueue already, so you will need to
write your own parser to convert your parameters into above structure.
Rakshasa
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Jari Sundell <hidden>
Date: 2006-08-23 12:55:52

On 8/23/06, Evgeniy Polyakov [off-list ref] wrote:
We still do not know what uintptr_t is, and it looks like it is a pointer,
which is forbidden. Those numbers are not enough to make network AIO.
And actually is not compatible with kqueue already, so you will need to
write your own parser to convert your parameters into above structure.
7.18.1.4 Integertypes capable of holding object pointers

"1 The following type designates a signed integer type with the
property that any valid
pointer to void can be converted to this type, then converted back to
pointer to void,
and the result will compare equal to the original pointer:"

Dunno if this means that x86-64 needs yet another typedef, or if using
long for intptr_t is incorrect. But assuming a different integer type
was used instead of intptr_t, that is known to be able to hold a
pointer, would there still be any problems?

I'm unable to see anything specific about AIO in your kevent patch
that these modifications wouldn't support.

Rakshasa

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 13:12:47

On Wed, Aug 23, 2006 at 02:55:47PM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
On 8/23/06, Evgeniy Polyakov [off-list ref] wrote:
quoted
We still do not know what uintptr_t is, and it looks like it is a pointer,
which is forbidden. Those numbers are not enough to make network AIO.
And actually is not compatible with kqueue already, so you will need to
write your own parser to convert your parameters into above structure.
7.18.1.4 Integertypes capable of holding object pointers

"1 The following type designates a signed integer type with the
property that any valid
pointer to void can be converted to this type, then converted back to
pointer to void,
and the result will compare equal to the original pointer:"

Dunno if this means that x86-64 needs yet another typedef, or if using
long for intptr_t is incorrect. But assuming a different integer type
was used instead of intptr_t, that is known to be able to hold a
pointer, would there still be any problems?
stdint.h

/* Types for `void *' pointers.  */
#if __WORDSIZE == 64
# ifndef __intptr_t_defined
typedef long int		intptr_t;
#  define __intptr_t_defined
# endif
typedef unsigned long int	uintptr_t;
#else
# ifndef __intptr_t_defined
typedef int			intptr_t;
#  define __intptr_t_defined
# endif
typedef unsigned int		uintptr_t;
#endif

which means that with 32bit userspace it will be equal to 32bit only.
I'm unable to see anything specific about AIO in your kevent patch
that these modifications wouldn't support.
I was asked to postpone AIO stuff for now, you can find it in previous
patchsets sent about week or two ago.
Rakshasa
-- 
	Evgeniy Polyakov

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Andrew Morton <hidden>
Date: 2006-08-23 16:10:42

On Wed, 23 Aug 2006 11:50:56 +0400
Evgeniy Polyakov [off-list ref] wrote:
On Wed, Aug 23, 2006 at 12:07:58AM -0700, Andrew Morton (akpm@osdl.org) wrote:
quoted
On Wed, 23 Aug 2006 10:56:59 +0400
Evgeniy Polyakov [off-list ref] wrote:
quoted
On Wed, Aug 23, 2006 at 02:43:50AM +0200, Jari Sundell (sundell.software@gmail.com) wrote:
quoted
Actually, I didn't miss that, it is an orthogonal issue. A timespec
timeout parameter for the syscall does not imply the use of timespec
in any timer event, etc. Nor is there any timespec timer in kqueue's
struct kevent, which is the only (interface related) thing that will
be exposed.
void * in structure exported to userspace is forbidden.
long in syscall requires wrapper in per-arch code (although that
workaround _is_ there, it does not mean that broken interface should 
be used).
poll uses millisecods - it is perfectly ok.
I wonder whether designing-in a millisecond granularity is the right thing
to do.  If in a few years the kernel is running tickless with high-res clock
interrupt sources, that might look a bit lumpy.

Switching it to a __u64 nanosecond counter would be basically free on
64-bit machines, and not very expensive on 32-bit, no?
I can put nanoseconds as timer interval too (with aligned_u64 as David
mentioned), and put it for timeout value too - 64 bit nanosecods ends up
with 58 years, probably enough.
Structures with u64 a really not so good idea.
OK.  One could do u32 seconds/u32 nsecs, but a simple aligned_u64 will be
better for 64-bit machines, and OK for 32-bit.

Re: [take12 0/3] kevent: Generic event handling mechanism.

From: Evgeniy Polyakov <hidden>
Date: 2006-08-23 16:23:22

On Wed, Aug 23, 2006 at 09:09:20AM -0700, Andrew Morton (akpm@osdl.org) wrote:
quoted
I can put nanoseconds as timer interval too (with aligned_u64 as David
mentioned), and put it for timeout value too - 64 bit nanosecods ends up
with 58 years, probably enough.
Structures with u64 a really not so good idea.
OK.  One could do u32 seconds/u32 nsecs, but a simple aligned_u64 will be
better for 64-bit machines, and OK for 32-bit.
aligned_u64 is not exported to userspace, so in the last patchset I just
use __u64 as syscall parameter.

-- 
	Evgeniy Polyakov
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help