[PATCH] Fix epoll backend to allow separate read/write events on one fd.
Stefan (metze) Metzmacher
metze at samba.org
Thu Feb 14 07:18:27 MST 2013
>> The tdgram_context and tstream_context abstractions only need one tevent_fd.
>> At least we should allow only one TEVENT_FD_READ handler and only one
>> TEVENT_FD_WRITE handler
>> for each fd.
> That is the idiom I think it's right to standardize on,
> and that's what my patch does.
I see the protection against more than two handlers.
>> But I'd think it's better to avoid additional complexity in
>> the epoll backend,
>> which is already complex.
> Yeah yeah - it's still a *lot* simpler than the poll backend,
> even *with* my multiplexing patch :-) :-).
Ok, looking at the patches your're right :-)
The tricky part is the EPOLL_ADDITIONAL_FD_FLAG_GOT_ERROR
and EPOLL_ADDITIONAL_FD_FLAG_REPORT_ERROR. I don't think that's
handled correct in your patches.
I'm also not sure if the EPOLL_ADDITIONAL_FD_FLAG_HAS_EVENT is filled
and cleared correctly.
Can you change the code to store the pointer to the other event
in fde->addtitional_data toghether with the
flag? That would avoid a list traverse for a lot of cases.
We need a destructor that removes the cross-reference on delete.
This way we can preferr the write handler over the read handler
and also make sure that tevent_fd_set_flags doesn't set the same flag
to both handlers.
> I'd be happy if you'd review, and I'm happy to fix any
> technical issues you might have, but this does need fixing
> to allow tevent to be usable over all backends.
It would be good to enforce the rule of just one fd event for
TEVENT_FD_READ and just one fd event for TEVENT_FD_WRITE.
We need to enforce this on tevent_add_fd() and on tevent_fd_set_flags()
and for all backends.
It's good to get this finally fixed, thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 261 bytes
Desc: OpenPGP digital signature
More information about the samba-technical