This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] epoll: Support for disabling items, and a self-testapp.


On 10/25/12 3:23 AM, Michael Kerrisk (man-pages) wrote:
Hi Pat,


I suppose that I have a concern that goes in the other direction. Is
there not some other solution possible that doesn't require the use of
EPOLLONESHOT? It seems overly restrictive to require that the caller
must employ this flag, and imposes the burden that the caller must
re-enable monitoring after each event.

Does a solution like the following (with no requirement for EPOLLONESHOT)
work?

0. Implement an epoll_ctl() operation EPOLL_CTL_XXX
    where the name XXX might be chosen based on the decision
    in 4(a).
1. EPOLL_CTL_XXX employs a private flag, EPOLLUSED, in the
    per-fd events mask in the ready list. By default,
    that flag is off.
2. epoll_wait() always clears the EPOLLUSED flag if a
    file descriptor is found to be ready.
3. If an epoll_ctl(EPOLL_CTL_XXX) discovers that the EPOLLUSED
    flag is NOT set, then
         a) it sets the EPOLLUSED flag
         b) It disables I/O events (as per EPOLL_CTL_DISABLE)
            (I'm not 100% sure if this is necesary).
         c) it returns EBUSY to the caller
4. If an epoll_ctl(EPOLL_CTL_XXX) discovers that the EPOLLUSED
    flag IS set, then it
         a) either deletes the fd or disables events for the fd
            (the choice here is a matter of design taste, I think;
            deletion has the virtue of simplicity; disabling provides
            the option to re-enable the fd later, if desired)
         b) returns 0 to the caller.

All of the above with suitable locking around the user-space cache.

Cheers,

Michael


I don't believe that proposal will solve the problem. Consider the case
where a worker thread has just executed epoll_wait and is about to execute
the next line of code (which will access the data associated with the fd
receiving the event). If the deletion thread manages to call
epoll_ctl(EPOLL_CTL_XXX) for that fd twice in a row before the worker thread
is able to execute the next statement, then the deletion thread will
mistakenly conclude that it is safe to destroy the data that the worker
thread is about to access.

Okay -- I had the idea there might be a hole in my proposal ;-).


By the way, have you been reading the comments in the two LWN articles
on EPOLL_CTL_DISABLE?
https://lwn.net/Articles/520012/
http://lwn.net/SubscriberLink/520198/fd81ba0ecb1858a2/

There's some interesting proposals there--some suggesting that an
entirely user-space solution might be possible. I haven't looked
deeply into the ideas though.

Yes, thanks, I read through the article and comments. I believe all of the objections raised there were either addressed by responses there, or they were also voiced here on the kernel.org mailing lists and addressed by either my or Paul Holland's responses here. If there is another objection to the original proposal that you feel I have overlooked or which has not been properly addressed, please let me know.


Pat


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]