[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

Hi Jeremy,

>> 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 :-)

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...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130214/bc42ffe0/attachment.pgp>

More information about the samba-technical mailing list