On Thu, Feb 28, 2008 at 2:12 PM, Michael Kerrisk
wrote:
>
> On Wed, Feb 27, 2008 at 8:35 PM, Davide Libenzi wrote:
> > On Tue, 26 Feb 2008, Chris "ã~B¯" Heath wrote:
> >
> > > On Tue, 2008-02-26 at 10:51 -0800, Davide Libenzi wrote:
> > > >
> >
> > > > Yes, you can't add the same fd twice. Think about a DB where "file*,fd" is
> > > > the key.
> > >
> > > To clarify, the key appears to be file* plus the user-space integer that
> > > represents the fd.
> >
> > Yes, that's what I said.
> >
> > > > > c) It is possible to add duplicated file descriptors referring to the same
> > > > > underlying open file description ("file *"). As you note, this can be a
> > > > > useful filtering technique, if the two file descriptors specify different
> > > > > masks.
> > > > >
> > > > > Assuming that is all correct, for man-pages-2.79, I've reworked the text
> > > > > for Q1/A1 as follows:
> > > > >
> > > > > Q1 What happens if you add the same file descriptor
> > > > > to an epoll set twice?
> > > > >
> > > > > A1 You will probably get EEXIST. However, it is pos-
> > > > > sible to add a duplicate (dup(2), dup2(2),
> > > > > fcntl(2) F_DUPFD, fork(2)) descriptor to the same
> > > > > epoll set. This can be a useful technique for
> > > > > filtering events, if the duplicate file descrip-
> > > > > tors are registered with different events masks.
> > > > >
> > > > > Seem okay Davide?
> > > >
> > > > Looks sane to me.
> > >
> > > I think fork(2) should not be in the above list. fork(2) duplicates the
> > > kernel's fd, but the user-space integer that represents the fd remains
> > > the same, so you will get EEXIST if you try to add the fd that was
> > > duplicated by fork.
> >
> > Good catch, fork(2) should not be there.
>
> Okay -- removed.
>
> But it is an ugly inconsistency. On the one hand, a child process
> cannot add the duplicate file descriptor to the epoll set. (In every
> other case that I can think of , descriptors duplicated by fork have
> similar semantics to descriptors duplicated by dup() and friends.) On
> the other hand, the very fact that the child has a duplicate of the
> descriptor means that even if the parent closes its descriptor, then
> epoll_wait() in the parent will continue to receive notifications for
> that descriptor because of the duplicated descriptor in the child.
>
> The choice of [file *, fd] as the key for epoll sets really does seem
> unfortunate. Keying on [pid, fd] would have given saner semantics, it
> seems to me. Obviously it can't be changed now though.


Davide,

with the earlier discussion in this thread in mind, I added a Q0/A0 to
epoll.7, just make the point about keys clear:


Q0 What is the key used to distinguish the file descrip-
tors in an epoll set?

A0 The key is the combination of the file descriptor
number and the open file description (also known as
"open file handle", the kernel's internal representa-
tion of an open file).

Does that seem okay?

Cheers,

Michael

--
Michael Kerrisk
Maintainer of the Linux man-pages project
http://www.kernel.org/doc/man-pages/
Want to report a man-pages bug? Look here:
http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
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/

On Thu, 28 Feb 2008, Michael Kerrisk wrote:

> But it is an ugly inconsistency. On the one hand, a child process
> cannot add the duplicate file descriptor to the epoll set. (In every
> other case that I can think of , descriptors duplicated by fork have
> similar semantics to descriptors duplicated by dup() and friends.) On
> the other hand, the very fact that the child has a duplicate of the
> descriptor means that even if the parent closes its descriptor, then
> epoll_wait() in the parent will continue to receive notifications for
> that descriptor because of the duplicated descriptor in the child.

Have you ever tried to think what it means for different *processes*
sharing a single epoll fd and doing epoll_wait() over it?
Most common case is a single event fetch thread plus dispatch. Going to
epoll_wait() over a single epoll fd from many *threads* is very much
possible, but requires care (news at 11, system software development
requires care too).
Sharing a single epoll fd (by the means of any process sharing it doing
add/wait) from different *processes* makes almost no sense at all.
"a child process cannot add the duplicate file descriptor to the epoll
set" ... how do you expect the parent (that doesn't even have the new fd
mapped) to react to such events?
If the next question is "But then why we made the epoll fd inheritable?",
the answer is, because it makes sense in many cases for a parent to hand
over an fd set to a child.



> The choice of [file *, fd] as the key for epoll sets really does seem
> unfortunate. Keying on [pid, fd] would have given saner semantics, it
> seems to me. Obviously it can't be changed now though.

I think we already went over this, and I think I clearly explained you the
reasons of not hooking into sys_close.



- Davide


--
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/

On Thu, 28 Feb 2008, Michael Kerrisk wrote:

> Davide,
>
> with the earlier discussion in this thread in mind, I added a Q0/A0 to
> epoll.7, just make the point about keys clear:
>
>
> Q0 What is the key used to distinguish the file descrip-
> tors in an epoll set?
>
> A0 The key is the combination of the file descriptor
> number and the open file description (also known as
> "open file handle", the kernel's internal representa-
> tion of an open file).
>
> Does that seem okay?

Looks fine to me! We need to clarify better Q6 WRT fork().



- Davide


--
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/

Hi Davide,

On Thu, Feb 28, 2008 at 8:23 PM, Davide Libenzi wrote:
> On Thu, 28 Feb 2008, Michael Kerrisk wrote:
>
> > But it is an ugly inconsistency. On the one hand, a child process
> > cannot add the duplicate file descriptor to the epoll set. (In every
> > other case that I can think of , descriptors duplicated by fork have
> > similar semantics to descriptors duplicated by dup() and friends.) On
> > the other hand, the very fact that the child has a duplicate of the
> > descriptor means that even if the parent closes its descriptor, then
> > epoll_wait() in the parent will continue to receive notifications for
> > that descriptor because of the duplicated descriptor in the child.
>
> Have you ever tried to think what it means for different *processes*
> sharing a single epoll fd and doing epoll_wait() over it?

As I think is clear, I've only given it very limited thought ;-).

The point is that the existing implementation actually supports
"different *processes* sharing a single epoll fd and doing
epoll_wait() over it", but the semantics are unintuitive. It may be
that the existing implementation was the best way of doing things.
But when I see the strange corner cases in the semantics, I can't help
but wonder (way too late), whether there might have been some other
way of implementing things that led to more intuitive semantics.

> Most common case is a single event fetch thread plus dispatch. Going to
> epoll_wait() over a single epoll fd from many *threads* is very much
> possible, but requires care (news at 11, system software development
> requires care too).
> Sharing a single epoll fd (by the means of any process sharing it doing
> add/wait) from different *processes* makes almost no sense at all.
>
> "a child process cannot add the duplicate file descriptor to the epoll
> set" ... how do you expect the parent (that doesn't even have the new fd
> mapped) to react to such events?

(Not sure if you missed my meaning here. Of course the parent already
has the fd mapped; it's the fd that the child inherited. Anyway, my
real point was that while descriptors duplicated by fork() are
normally semantically similar to other duplicated descriptors, when it
comes to epoll they are not -- and that has the potential to surprise
users.)

> If the next question is "But then why we made the epoll fd inheritable?",
> the answer is, because it makes sense in many cases for a parent to hand
> over an fd set to a child.

Fair enough.

So here's an idea about how things might alternatively have been done:

a) The key for epoll entries could have been [file *, fd, PID]

b) an epoll_wait() only returns events for fds where the PID maps that
of the caller.

c) a close of a file descriptor removes the corresponding [file *,
fd, PID] from the epoll set.

d) when a fork() is done, then the epoll set has a new set of keys
added. These are duplicates of the [file *, fd, PID] entries for the
parent, but with the PID of the child substituted into the new keys.
Say the parent had PID 1000, and the child has PID 2000. If the epoll
set initially contained:

[X, 3, 1000]
[Y, 4, 1000]

then after fork() we'd have:

[X, 3, 1000]
[Y, 4, 1000]
[X, 3, 2000]
[Y, 4, 2000]

There is of course room for debate about the efficiency of this
approach, I suppose. But it seems to me (and perhaps I've missed a
number of things) that that could have given sane semantics with
respect to fork(), duplicated descriptors, and close(). Furthermore,
it would have allowed us to sanely support "different *processes*
sharing a single epoll fd and doing epoll_wait() over it".

Of course, this is all academic now: we can't change the ABI.

> > The choice of [file *, fd] as the key for epoll sets really does seem
> > unfortunate. Keying on [pid, fd] would have given saner semantics, it
> > seems to me. Obviously it can't be changed now though.
>
> I think we already went over this, and I think I clearly explained you the
> reasons of not hooking into sys_close.

You said elsewhere:

[[
That'd mean placing an eventpoll custom hook into sys_close(). Looks very
bad to me, and probably will look even worse to other kernel folks.
Is not much a performance issue (a check to see if a file* is an eventpoll
file is as easy as comparing the f_op pointer), but a design/style issue.
]]

But that wasn't very clear to me actually. I note that filp_close()
already has special case handling for dnotify (R.I.P.) and fcntl()
)aka POSIX) file locks, so there was already precedent for a custom
hook, AFAICS, and epoll is at least as worthy of special treatment as
either of those cases.

Cheers,

Michael

--
Michael Kerrisk
Maintainer of the Linux man-pages project
http://www.kernel.org/doc/man-pages/
Want to report a man-pages bug? Look here:
http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
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/

On Fri, 29 Feb 2008, Michael Kerrisk wrote:

> As I think is clear, I've only given it very limited thought ;-).
>
> The point is that the existing implementation actually supports
> "different *processes* sharing a single epoll fd and doing
> epoll_wait() over it", but the semantics are unintuitive. It may be
> that the existing implementation was the best way of doing things.
> But when I see the strange corner cases in the semantics, I can't help
> but wonder (way too late), whether there might have been some other
> way of implementing things that led to more intuitive semantics.

Oh boy. The fact that you can have an epoll fd cross the fork boundary,
does not mean that any indiscriminate use of it leads to sane results:

efd = epoll_create();
fork();
pipe(fds);
epoll_ctl(efd, ADD, fds[0]);
epoll_wait(); ????
...
pipe(fds);
epoll_ctl(efd, ADD, fds[0]);
epoll_wait(); ????


It is *NOT* a matter of semantics.



> > If the next question is "But then why we made the epoll fd inheritable?",
> > the answer is, because it makes sense in many cases for a parent to hand
> > over an fd set to a child.
>
> Fair enough.
>
> So here's an idea about how things might alternatively have been done:
>
> a) The key for epoll entries could have been [file *, fd, PID]
>
> b) an epoll_wait() only returns events for fds where the PID maps that
> of the caller.
>
> c) a close of a file descriptor removes the corresponding [file *,
> fd, PID] from the epoll set.
>
> d) when a fork() is done, then the epoll set has a new set of keys
> added. These are duplicates of the [file *, fd, PID] entries for the
> parent, but with the PID of the child substituted into the new keys.
> Say the parent had PID 1000, and the child has PID 2000. If the epoll
> set initially contained:
>
> [X, 3, 1000]
> [Y, 4, 1000]
>
> then after fork() we'd have:
>
> [X, 3, 1000]
> [Y, 4, 1000]
> [X, 3, 2000]
> [Y, 4, 2000]
>
> There is of course room for debate about the efficiency of this
> approach, I suppose.

There sure is



> You said elsewhere:
>
> [[
> That'd mean placing an eventpoll custom hook into sys_close(). Looks very
> bad to me, and probably will look even worse to other kernel folks.
> Is not much a performance issue (a check to see if a file* is an eventpoll
> file is as easy as comparing the f_op pointer), but a design/style issue.
> ]]
>
> But that wasn't very clear to me actually. I note that filp_close()
> already has special case handling for dnotify (R.I.P.) and fcntl()
> )aka POSIX) file locks, so there was already precedent for a custom
> hook, AFAICS, and epoll is at least as worthy of special treatment as
> either of those cases.

I guess that over the time, Al became software WRT junk going there



- Davide


--
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/

On Fri, Feb 29, 2008 at 8:19 PM, Davide Libenzi wrote:
> On Fri, 29 Feb 2008, Michael Kerrisk wrote:
>
> > As I think is clear, I've only given it very limited thought ;-).
> >
> > The point is that the existing implementation actually supports
> > "different *processes* sharing a single epoll fd and doing
> > epoll_wait() over it", but the semantics are unintuitive. It may be
> > that the existing implementation was the best way of doing things.
> > But when I see the strange corner cases in the semantics, I can't help
> > but wonder (way too late), whether there might have been some other
> > way of implementing things that led to more intuitive semantics.
>
> Oh boy. The fact that you can have an epoll fd cross the fork boundary,
> does not mean that any indiscriminate use of it leads to sane results:

I ddidn't mean that it did. Certainly in the current implementation
it there will insane situations ;-).

> efd = epoll_create();
> fork();
> pipe(fds);
> epoll_ctl(efd, ADD, fds[0]);
> epoll_wait(); ????
> ...
> pipe(fds);
> epoll_ctl(efd, ADD, fds[0]);
> epoll_wait(); ????
>
>
> It is *NOT* a matter of semantics.

Of course -- but I don't think I suggested that I disagree on this.

> > > If the next question is "But then why we made the epoll fd inheritable?",
> > > the answer is, because it makes sense in many cases for a parent to hand
> > > over an fd set to a child.
> >
> > Fair enough.
> >
> > So here's an idea about how things might alternatively have been done:
> >
> > a) The key for epoll entries could have been [file *, fd, PID]
> >
> > b) an epoll_wait() only returns events for fds where the PID maps that
> > of the caller.
> >
> > c) a close of a file descriptor removes the corresponding [file *,
> > fd, PID] from the epoll set.
> >
> > d) when a fork() is done, then the epoll set has a new set of keys
> > added. These are duplicates of the [file *, fd, PID] entries for the
> > parent, but with the PID of the child substituted into the new keys.
> > Say the parent had PID 1000, and the child has PID 2000. If the epoll
> > set initially contained:
> >
> > [X, 3, 1000]
> > [Y, 4, 1000]
> >
> > then after fork() we'd have:
> >
> > [X, 3, 1000]
> > [Y, 4, 1000]
> > [X, 3, 2000]
> > [Y, 4, 2000]
> >
> > There is of course room for debate about the efficiency of this
> > approach, I suppose.
>
> There sure is

Okay -- but I suspect it could have been made fairly efficient.

> > You said elsewhere:
> >
> > [[
> > That'd mean placing an eventpoll custom hook into sys_close(). Looks very
> > bad to me, and probably will look even worse to other kernel folks.
> > Is not much a performance issue (a check to see if a file* is an eventpoll
> > file is as easy as comparing the f_op pointer), but a design/style issue.
> > ]]
> >
> > But that wasn't very clear to me actually. I note that filp_close()
> > already has special case handling for dnotify (R.I.P.) and fcntl()
> > )aka POSIX) file locks, so there was already precedent for a custom
> > hook, AFAICS, and epoll is at least as worthy of special treatment as
> > either of those cases.
>
> I guess that over the time, Al became software WRT junk going there

Sorry -- I don't understand that last sentence?
--
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/

Hijacking this epoll thread, the following related question occurs to me:

#Q8
# Does an operation on a file descriptor affect the already collected but
#not yet reported events?
#
#A8
# You can do two operations on an existing file descriptor. Remove would
#be meaningless for this case. Modify will re-read available I/O.

Why is EPOLL_CTL_DEL considered meaningless? A process is wrapping up its
business and is preparing to remove the fd from the epoll set, and then
close the file descriptor itself. In the meantime, the fd became readable,
and a POLLIN event gets collected. So, what happens to the collected event,
when the EPOLL_CTL_DEL operation is made?


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQBHysO9x9p3GYHlUOIRAqEQAJ9QcE4nl6OQ3ZxOXDdM7M r9ZpXBWQCfSZwp
bxUJC9q3agGsE9Kd4EkV/w8=
=JtXf
-----END PGP SIGNATURE-----

On Sun, 2 Mar 2008, Sam Varshavchik wrote:

> Hijacking this epoll thread, the following related question occurs to me:
>
> #Q8
> # Does an operation on a file descriptor affect the already collected but
> #not yet reported events?
> #
> #A8
> # You can do two operations on an existing file descriptor. Remove would
> #be meaningless for this case. Modify will re-read available I/O.
>
> Why is EPOLL_CTL_DEL considered meaningless? A process is wrapping up its
> business and is preparing to remove the fd from the epoll set, and then close
> the file descriptor itself. In the meantime, the fd became readable, and a
> POLLIN event gets collected. So, what happens to the collected event, when the
> EPOLL_CTL_DEL operation is made?

Any epoll_wait() done after the POLLIN and before the EPOLL_CTL_DEL, will
show up. After the EPOLL_CTL_DEL, of course, no events will be reported.



- Davide


--
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/