This adds a new system call, epoll_mod_wait. It's described as below:
NAME
epoll_mod_wait - modify and wait for I/O events on an epoll file
descriptor
SYNOPSIS
int epoll_mod_wait(int epfd, int flags,
int ncmds, struct epoll_mod_cmd *cmds,
struct epoll_wait_spec *spec);
DESCRIPTION
The epoll_mod_wait() system call can be seen as an enhanced combination
of several epoll_ctl(2) calls, which are followed by an epoll_pwait(2)
call. It is superior in two cases:
1) When epoll_ctl(2) are followed by epoll_wait(2), using epoll_mod_wait
will save context switches between user mode and kernel mode;
2) When you need higher precision than microsecond for wait timeout.
The epoll_ctl(2) operations are embedded into this call by with ncmds
and cmds. The latter is an array of command structs:
struct epoll_mod_cmd {
/* Reserved flags for future extension, must be 0 for now. */
int flags;
/* The same as epoll_ctl() op parameter. */
int op;
/* The same as epoll_ctl() fd parameter. */
int fd;
/* The same as the "events" field in struct epoll_event. */
uint32_t events;
/* The same as the "data" field in struct epoll_event. */
uint64_t data;
/* Output field, will be set to the return code once this
* command is executed by kernel */
int error;
};
There is no guartantee that all the commands are executed in order. Only
if all the commands are successfully executed (all the error fields are
set to 0), events are polled.
The last parameter "spec" is a pointer to struct epoll_wait_spec, which
contains the information about how to poll the events. If it's NULL, this
call will immediately return after running all the commands in cmds.
The structure is defined as below:
struct epoll_wait_spec {
/* The same as "maxevents" in epoll_pwait() */
int maxevents;
/* The same as "events" in epoll_pwait() */
struct epoll_event *events;
/* Which clock to use for timeout */
int clockid;
/* Maximum time to wait if there is no event */
struct timespec timeout;
/* The same as "sigmask" in epoll_pwait() */
sigset_t *sigmask;
/* The same as "sigsetsize" in epoll_pwait() */
size_t sigsetsize;
} EPOLL_PACKED;
RETURN VALUE
When any error occurs, epoll_mod_wait() returns -1 and errno is set
appropriately. All the "error" fields in cmds are unchanged before they
are executed, and if any cmds are executed, the "error" fields are set
to a return code accordingly. See also epoll_ctl for more details of the
return code.
When successful, epoll_mod_wait() returns the number of file
descriptors ready for the requested I/O, or zero if no file descriptor
became ready during the requested timeout milliseconds.
If spec is NULL, it returns 0 if all the commands are successful, and -1
if an error occured.
ERRORS
These errors apply on either the return value of epoll_mod_wait or error
status for each command, respectively.
EBADF epfd or fd is not a valid file descriptor.
EFAULT The memory area pointed to by events is not accessible with write
permissions.
EINTR The call was interrupted by a signal handler before either any of
the requested events occurred or the timeout expired; see
signal(7).
EINVAL epfd is not an epoll file descriptor, or maxevents is less than
or equal to zero, or fd is the same as epfd, or the requested
operation op is not supported by this interface.
EEXIST op was EPOLL_CTL_ADD, and the supplied file descriptor fd is
already registered with this epoll instance.
ENOENT op was EPOLL_CTL_MOD or EPOLL_CTL_DEL, and fd is not registered
with this epoll instance.
ENOMEM There was insufficient memory to handle the requested op control
operation.
ENOSPC The limit imposed by /proc/sys/fs/epoll/max_user_watches was
encountered while trying to register (EPOLL_CTL_ADD) a new file
descriptor on an epoll instance. See epoll(7) for further
details.
EPERM The target file fd does not support epoll.
CONFORMING TO
epoll_mod_wait() is Linux-specific.
SEE ALSO
epoll_create(2), epoll_ctl(2), epoll_wait(2), epoll_pwait(2), epoll(7)
Fam Zheng (6):
epoll: Extract epoll_wait_do and epoll_pwait_do
epoll: Specify clockid explicitly
epoll: Add definition for epoll_mod_wait structures
epoll: Extract ep_ctl_do
epoll: Add implementation for epoll_mod_wait
x86: Hook up epoll_mod_wait syscall
arch/x86/syscalls/syscall_32.tbl | 1 +
arch/x86/syscalls/syscall_64.tbl | 1 +
fs/eventpoll.c | 219 +++++++++++++++++++++++++--------------
include/linux/syscalls.h | 5 +
include/uapi/linux/eventpoll.h | 20 ++++
5 files changed, 167 insertions(+), 79 deletions(-)
--
1.9.3
This syscall is a sequence of
1) a number of epoll_ctl calls
2) a epoll_pwait, with timeout enhancement.
The epoll_ctl operations are embeded so that application doesn't have to use
separate syscalls to insert/delete/update the fds before poll. It is more
efficient if the set of fds varies from one poll to another, which is the
common pattern for certain applications. For example, depending on the input
buffer status, a data reading program may decide to temporarily not polling an
fd.
Because the enablement of batching in this interface, even that regular
epoll_ctl call sequence, which manipulates several fds, can be optimized to one
single epoll_ctl_wait (while specifying spec=NULL to skip the poll part).
The only complexity is returning the result of each operation. For each
epoll_mod_cmd in cmds, the field "error" is an output field that will be stored
the return code *iff* the command is executed (0 for success and -errno of the
equivalent epoll_ctl call), and will be left unchanged if the command is not
executed because some earlier error, for example due to failure of
copy_from_user to copy the array.
Applications can utilize this fact to do error handling: they could initialize
all the epoll_mod_wait.error to a positive value, which is by definition not a
possible output value from epoll_mod_wait. Then when the syscall returned, they
know whether or not the command is executed by comparing each error with the
init value, if they're different, they have the result of the command.
More roughly, they can put any non-zero and not distinguish "not run" from
failure.
Also, timeout parameter is enhanced: timespec is used, compared to the old ms
scalar. This provides higher precision. The parameter field in struct
epoll_wait_spec, "clockid", also makes it possible for users to use a different
clock than the default when it makes more sense.
Signed-off-by: Fam Zheng <redacted>
---
fs/eventpoll.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/syscalls.h | 5 ++++
2 files changed, 65 insertions(+)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index e7a116d..2cc22c9 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -2067,6 +2067,66 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events,
sigmask ? &ksigmask : NULL);
}
+SYSCALL_DEFINE5(epoll_mod_wait, int, epfd, int, flags,
+ int, ncmds, struct epoll_mod_cmd __user *, cmds,
+ struct epoll_wait_spec __user *, spec)
+{
+ struct epoll_mod_cmd *kcmds = NULL;
+ int i, ret = 0;
+ int cmd_size = sizeof(struct epoll_mod_cmd) * ncmds;
+
+ if (flags)
+ return -EINVAL;
+ if (ncmds) {
+ if (!cmds)
+ return -EINVAL;
+ kcmds = kmalloc(cmd_size, GFP_KERNEL);
+ if (!kcmds)
+ return -ENOMEM;
+ if (copy_from_user(kcmds, cmds, cmd_size)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ }
+ for (i = 0; i < ncmds; i++) {
+ struct epoll_event ev = (struct epoll_event) {
+ .events = kcmds[i].events,
+ .data = kcmds[i].data,
+ };
+ if (kcmds[i].flags) {
+ kcmds[i].error = ret = -EINVAL;
+ goto out;
+ }
+ kcmds[i].error = ret = ep_ctl_do(epfd, kcmds[i].op, kcmds[i].fd, ev);
+ if (ret)
+ goto out;
+ }
+ if (spec) {
+ sigset_t ksigmask;
+ struct epoll_wait_spec kspec;
+ ktime_t timeout;
+
+ if(copy_from_user(&kspec, spec, sizeof(struct epoll_wait_spec)))
+ return -EFAULT;
+ if (kspec.sigmask) {
+ if (kspec.sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+ if (copy_from_user(&ksigmask, kspec.sigmask, sizeof(ksigmask)))
+ return -EFAULT;
+ }
+ timeout = timespec_to_ktime(kspec.timeout);
+ ret = epoll_pwait_do(epfd, kspec.events, kspec.maxevents,
+ kspec.clockid, timeout,
+ kspec.sigmask ? &ksigmask : NULL);
+ }
+
+out:
+ if (ncmds && copy_to_user(cmds, kcmds, cmd_size))
+ return -EFAULT;
+ kfree(kcmds);
+ return ret;
+}
+
#ifdef CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE6(epoll_pwait, int, epfd,
struct epoll_event __user *, events,diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 85893d7..7156c80 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -12,6 +12,8 @@
#define _LINUX_SYSCALLS_H
struct epoll_event;
+struct epoll_mod_cmd;
+struct epoll_wait_spec;
struct iattr;
struct inode;
struct iocb;
@@ -630,6 +632,9 @@ asmlinkage long sys_epoll_pwait(int epfd, struct epoll_event __user *events,
int maxevents, int timeout,
const sigset_t __user *sigmask,
size_t sigsetsize);
+asmlinkage long sys_epoll_mod_wait(int epfd, int flags,
+ int ncmds, struct epoll_mod_cmd __user * cmds,
+ struct epoll_wait_spec __user * spec);
asmlinkage long sys_gethostname(char __user *name, int len);
asmlinkage long sys_sethostname(char __user *name, int len);
asmlinkage long sys_setdomainname(char __user *name, int len);
--
1.9.3
Hello Fam Zheng,
On 01/20/2015 10:57 AM, Fam Zheng wrote:
This syscall is a sequence of
1) a number of epoll_ctl calls
2) a epoll_pwait, with timeout enhancement.
The epoll_ctl operations are embeded so that application doesn't have to use
separate syscalls to insert/delete/update the fds before poll. It is more
efficient if the set of fds varies from one poll to another, which is the
common pattern for certain applications.
Which applications? Could we have some specific examples? This is a
complex API, and it needs good justification.
For example, depending on the input
buffer status, a data reading program may decide to temporarily not polling an
fd.
Because the enablement of batching in this interface, even that regular
epoll_ctl call sequence, which manipulates several fds, can be optimized to one
single epoll_ctl_wait (while specifying spec=NULL to skip the poll part).
^^^^^^^^^^^^^^ should be epoll_mod_wait
I think you mean to say:
The ability to batch multiple "epoll_ctl" operations into a single call
means that even when no wait events are requested (i.e., spec == NULL),
poll_mod_wait() provides a performance optimization over using multiple
epoll_ctl() calls.
Right? If yes, please amend the commit message, and this text should
also make its way into the revised man page under a heading "NOTES".
The only complexity is returning the result of each operation. For each
epoll_mod_cmd in cmds, the field "error" is an output field that will be stored
the return code *iff* the command is executed (0 for success and -errno of the
equivalent epoll_ctl call), and will be left unchanged if the command is not
executed because some earlier error, for example due to failure of
copy_from_user to copy the array.
Applications can utilize this fact to do error handling: they could initialize
all the epoll_mod_wait.error to a positive value, which is by definition not a
possible output value from epoll_mod_wait. Then when the syscall returned, they
know whether or not the command is executed by comparing each error with the
init value, if they're different, they have the result of the command.
More roughly, they can put any non-zero and not distinguish "not run" from
failure.
The "cmds' are not executed in a specified order plus the need to
initialize the 'errors' fields to a positive value feels a bit ugly.
And indeed the whole "command list was only partially run" case
is not pretty. Am I correct to understand that if an error is found
during execution of one of the "epoll_ctl" commands in 'cmds' then
the system call will return -1 with errno set, indicating an error,
even though the epoll interest list may have changed because some
of the earlier 'cmds' executed successfully? This all seems a bit of
a headache for user space.
I have a couple of questions:
Q1. I can see that batching "epoll_ctl" commands might be useful,
since it results in fewer systems calls. But, does it really
need to be bound together with the "epoll_pwait" functionality?
(Perhaps this point was covered in previous discussions, but
neither the message accompanying this patch nor the 0/6 man page
provide a compelling rationale for the need to bind these two
operations together.)
Yes, I realize you might save a system call, but it makes for a
cumbersome API that has the above headache, and also forces the
need for double pointer indirection in the 'spec' argument (i.e.,
spec is a pointer to an array of structures where each element
in turn includes an 'events' pointer that points to another array).
Why not a simpler API with two syscalls such as:
epoll_ctl_batch(int epfd, int flags,
int ncmds, struct epoll_mod_cmd *cmds);
epoll_pwait1(int epfd, struct epoll_event *events, int maxevents,
struct timespec *timeout, int clock_id,
const sigset_t *sigmask, size_t sigsetsize);
This gives us much of the benefit of reducing system calls, but
with greater simplicity. And epoll_ctl_batch() could simply return
the number of 'cmds' that were successfully executed.)
Q2. In the man page in 0/6 you said that the 'cmds' were not
guaranteed to be executed in order. Why not? If you did provide
such a guarantee, then, when using your current epoll_mod_wait(),
user space could do the following:
1. Initialize the cmd.errors fields to zero.
2. Call epoll_ctl_mod()
3. Iterate through cmd.errors looking for the first nonzero
field.
Also, timeout parameter is enhanced: timespec is used, compared to the old ms
scalar. This provides higher precision.
Yes, that change seemed inevitable. It slightly puzzled me at the time when
Davide Libenzi added epoll_wait() that the timeout was milliseconds, even
though pselect() already had demonstrated the need for higher precision.
I should have called it out way back then :-{.
quoted hunk
The parameter field in struct
epoll_wait_spec, "clockid", also makes it possible for users to use a different
clock than the default when it makes more sense.
Signed-off-by: Fam Zheng <redacted>
---
fs/eventpoll.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/syscalls.h | 5 ++++
2 files changed, 65 insertions(+)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index e7a116d..2cc22c9 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -2067,6 +2067,66 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events,
sigmask ? &ksigmask : NULL);
}
+SYSCALL_DEFINE5(epoll_mod_wait, int, epfd, int, flags,
+ int, ncmds, struct epoll_mod_cmd __user *, cmds,
+ struct epoll_wait_spec __user *, spec)
+{
+ struct epoll_mod_cmd *kcmds = NULL;
+ int i, ret = 0;
+ int cmd_size = sizeof(struct epoll_mod_cmd) * ncmds;
+
+ if (flags)
+ return -EINVAL;
+ if (ncmds) {
+ if (!cmds)
+ return -EINVAL;
+ kcmds = kmalloc(cmd_size, GFP_KERNEL);
+ if (!kcmds)
+ return -ENOMEM;
+ if (copy_from_user(kcmds, cmds, cmd_size)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ }
+ for (i = 0; i < ncmds; i++) {
+ struct epoll_event ev = (struct epoll_event) {
+ .events = kcmds[i].events,
+ .data = kcmds[i].data,
+ };
+ if (kcmds[i].flags) {
+ kcmds[i].error = ret = -EINVAL;
To make the 'ret' change a little more obvious, maybe it's better to write
ret = kcmds[i].error = -EINVAL;
+ goto out;
+ }
+ kcmds[i].error = ret = ep_ctl_do(epfd, kcmds[i].op, kcmds[i].fd, ev);
Likewise:
ret = kcmds[i].error = ep_ctl_do(epfd, kcmds[i].op, kcmds[i].fd, ev);
+ if (ret)
+ goto out;
+ }
+ if (spec) {
+ sigset_t ksigmask;
+ struct epoll_wait_spec kspec;
+ ktime_t timeout;
+
+ if(copy_from_user(&kspec, spec, sizeof(struct epoll_wait_spec)))
Cosmetic point: s/if(/if (/
+ return -EFAULT;
+ if (kspec.sigmask) {
+ if (kspec.sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+ if (copy_from_user(&ksigmask, kspec.sigmask, sizeof(ksigmask)))
+ return -EFAULT;
+ }
+ timeout = timespec_to_ktime(kspec.timeout);
+ ret = epoll_pwait_do(epfd, kspec.events, kspec.maxevents,
+ kspec.clockid, timeout,
+ kspec.sigmask ? &ksigmask : NULL);
If I understand correctly, the implementation means that the
'size_t sigsetsize' field will probably need to be exposed to
applications. In the existing epoll_pwait() call (as in ppoll()
and pselect()) the 'size_t sigsetsize' argument is hidden by glibc.
However, unless we expect glibc to do some structure copying to/from
a structure that hides this field, then we're going end up exposing
'size_t sigsetsize' to applications. (This could be avoided, if we
split the API as I suggest above. glibc would do the same thing
in epoll_pwait1() that it currently does in epoll_pwait().)
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On Tue, 01/20 13:50, Michael Kerrisk (man-pages) wrote:
Hello Fam Zheng,
On 01/20/2015 10:57 AM, Fam Zheng wrote:
quoted
This syscall is a sequence of
1) a number of epoll_ctl calls
2) a epoll_pwait, with timeout enhancement.
The epoll_ctl operations are embeded so that application doesn't have to use
separate syscalls to insert/delete/update the fds before poll. It is more
efficient if the set of fds varies from one poll to another, which is the
common pattern for certain applications.
Which applications? Could we have some specific examples? This is a
complex API, and it needs good justification.
OK, I'll explain more in v2.
quoted
For example, depending on the input
buffer status, a data reading program may decide to temporarily not polling an
fd.
Because the enablement of batching in this interface, even that regular
epoll_ctl call sequence, which manipulates several fds, can be optimized to one
single epoll_ctl_wait (while specifying spec=NULL to skip the poll part).
^^^^^^^^^^^^^^ should be epoll_mod_wait
I think you mean to say:
The ability to batch multiple "epoll_ctl" operations into a single call
means that even when no wait events are requested (i.e., spec == NULL),
poll_mod_wait() provides a performance optimization over using multiple
epoll_ctl() calls.
Right? If yes, please amend the commit message, and this text should
also make its way into the revised man page under a heading "NOTES".
OK.
quoted
The only complexity is returning the result of each operation. For each
epoll_mod_cmd in cmds, the field "error" is an output field that will be stored
the return code *iff* the command is executed (0 for success and -errno of the
equivalent epoll_ctl call), and will be left unchanged if the command is not
executed because some earlier error, for example due to failure of
copy_from_user to copy the array.
Applications can utilize this fact to do error handling: they could initialize
all the epoll_mod_wait.error to a positive value, which is by definition not a
possible output value from epoll_mod_wait. Then when the syscall returned, they
know whether or not the command is executed by comparing each error with the
init value, if they're different, they have the result of the command.
More roughly, they can put any non-zero and not distinguish "not run" from
failure.
The "cmds' are not executed in a specified order plus the need to
initialize the 'errors' fields to a positive value feels a bit ugly.
And indeed the whole "command list was only partially run" case
is not pretty. Am I correct to understand that if an error is found
during execution of one of the "epoll_ctl" commands in 'cmds' then
the system call will return -1 with errno set, indicating an error,
even though the epoll interest list may have changed because some
of the earlier 'cmds' executed successfully? This all seems a bit of
a headache for user space.
This is the trade-off for batching. The best we can do is probably make this
transactional: none or all of the commands succeeds. It will require a much
more complex implementation, though. But even with that, the error reporting on
which command failed is a complication.
I have a couple of questions:
Q1. I can see that batching "epoll_ctl" commands might be useful,
since it results in fewer systems calls. But, does it really
need to be bound together with the "epoll_pwait" functionality?
(Perhaps this point was covered in previous discussions, but
neither the message accompanying this patch nor the 0/6 man page
provide a compelling rationale for the need to bind these two
operations together.)
Yes, I realize you might save a system call, but it makes for a
cumbersome API that has the above headache, and also forces the
need for double pointer indirection in the 'spec' argument (i.e.,
spec is a pointer to an array of structures where each element
in turn includes an 'events' pointer that points to another array).
Why not a simpler API with two syscalls such as:
epoll_ctl_batch(int epfd, int flags,
int ncmds, struct epoll_mod_cmd *cmds);
epoll_pwait1(int epfd, struct epoll_event *events, int maxevents,
struct timespec *timeout, int clock_id,
const sigset_t *sigmask, size_t sigsetsize);
The problem is that there is no room for flags field in epoll_pwait1, which is
asked for, in previous discussion thread [1].
I don't see epoll_mod_wait as a *significantly more* complicated interface
compared to epoll_ctl_batch and epoll_pwait1 above. In epoll_mod_wait, if you
leave out ncmds and cmds, it is effectively a poll without batch; and if
leaving out spec, it is effectively a batch without poll.
The most important change here is the timeout. IMO I wouldn't mind leaving out
batching. Integrating it is requested by Andy:
[1]: http://thread.gmane.org/gmane.linux.kernel/1861430/focus=91591
which also made sense to me; I do have a patch in QEMU to call epoll_ctl for a
number of times right before epoll_wait.
[Sorry for not putting anything into cover letter changelog, but it is also
interesting to see people's reaction on the patch itself without bias of
others' opinions. This indeed brings in more points. :]
This gives us much of the benefit of reducing system calls, but
with greater simplicity. And epoll_ctl_batch() could simply return
the number of 'cmds' that were successfully executed.)
Q2. In the man page in 0/6 you said that the 'cmds' were not
guaranteed to be executed in order. Why not? If you did provide
such a guarantee, then, when using your current epoll_mod_wait(),
user space could do the following:
I guess we can make a guarentee on that.
1. Initialize the cmd.errors fields to zero.
2. Call epoll_ctl_mod()
3. Iterate through cmd.errors looking for the first nonzero
field.
It's close, but zero is not good enough, if copy_from_user of cmds failed in
the first place. Impossible value, or error value, will be safer.
quoted
Also, timeout parameter is enhanced: timespec is used, compared to the old ms
scalar. This provides higher precision.
Yes, that change seemed inevitable. It slightly puzzled me at the time when
Davide Libenzi added epoll_wait() that the timeout was milliseconds, even
though pselect() already had demonstrated the need for higher precision.
I should have called it out way back then :-{.
quoted
The parameter field in struct
epoll_wait_spec, "clockid", also makes it possible for users to use a different
clock than the default when it makes more sense.
Signed-off-by: Fam Zheng <redacted>
---
fs/eventpoll.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/syscalls.h | 5 ++++
2 files changed, 65 insertions(+)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index e7a116d..2cc22c9 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -2067,6 +2067,66 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events,
sigmask ? &ksigmask : NULL);
}
+SYSCALL_DEFINE5(epoll_mod_wait, int, epfd, int, flags,
+ int, ncmds, struct epoll_mod_cmd __user *, cmds,
+ struct epoll_wait_spec __user *, spec)
+{
+ struct epoll_mod_cmd *kcmds = NULL;
+ int i, ret = 0;
+ int cmd_size = sizeof(struct epoll_mod_cmd) * ncmds;
+
+ if (flags)
+ return -EINVAL;
+ if (ncmds) {
+ if (!cmds)
+ return -EINVAL;
+ kcmds = kmalloc(cmd_size, GFP_KERNEL);
+ if (!kcmds)
+ return -ENOMEM;
+ if (copy_from_user(kcmds, cmds, cmd_size)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ }
+ for (i = 0; i < ncmds; i++) {
+ struct epoll_event ev = (struct epoll_event) {
+ .events = kcmds[i].events,
+ .data = kcmds[i].data,
+ };
+ if (kcmds[i].flags) {
+ kcmds[i].error = ret = -EINVAL;
To make the 'ret' change a little more obvious, maybe it's better to write
ret = kcmds[i].error = -EINVAL;
quoted
+ goto out;
+ }
+ kcmds[i].error = ret = ep_ctl_do(epfd, kcmds[i].op, kcmds[i].fd, ev);
Likewise:
ret = kcmds[i].error = ep_ctl_do(epfd, kcmds[i].op, kcmds[i].fd, ev);
quoted
+ if (ret)
+ goto out;
+ }
+ if (spec) {
+ sigset_t ksigmask;
+ struct epoll_wait_spec kspec;
+ ktime_t timeout;
+
+ if(copy_from_user(&kspec, spec, sizeof(struct epoll_wait_spec)))
Cosmetic point: s/if(/if (/
quoted
+ return -EFAULT;
+ if (kspec.sigmask) {
+ if (kspec.sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+ if (copy_from_user(&ksigmask, kspec.sigmask, sizeof(ksigmask)))
+ return -EFAULT;
+ }
+ timeout = timespec_to_ktime(kspec.timeout);
+ ret = epoll_pwait_do(epfd, kspec.events, kspec.maxevents,
+ kspec.clockid, timeout,
+ kspec.sigmask ? &ksigmask : NULL);
If I understand correctly, the implementation means that the
'size_t sigsetsize' field will probably need to be exposed to
applications. In the existing epoll_pwait() call (as in ppoll()
and pselect()) the 'size_t sigsetsize' argument is hidden by glibc.
However, unless we expect glibc to do some structure copying to/from
a structure that hides this field, then we're going end up exposing
'size_t sigsetsize' to applications. (This could be avoided, if we
split the API as I suggest above. glibc would do the same thing
in epoll_pwait1() that it currently does in epoll_pwait().)
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/