On Thu, Jun 23, 2016 at 5:40 PM, Willem Jan Withagen <wjw@xxxxxxxxxxx> wrote: > On 23-6-2016 09:44, Haomai Wang wrote: >> On Thu, Jun 23, 2016 at 3:22 PM, Willem Jan Withagen <wjw@xxxxxxxxxxx> wrote: >>> On 23-6-2016 00:16, Willem Jan Withagen wrote: >>>> On 22-6-2016 16:36, Haomai Wang wrote: >>>>> Oh, sorry. I still realize you are testing on kqueue event backend. >>>>> >>>>> I submit a pr to fix this. plz help to verify whether it works for you >>>>> since I don't have bsd handy... >>>>> >>>>> https://github.com/ceph/ceph/pull/9869 >>>> >>>> I think add_event needs about the same treatment. >>>> Trying, Testing ATM.... >>> >>> errgh, not quite... >>> >>> It also generates errors when trying to delete EVFILT_READ (mask=2) from >>> an eventfilter that has both READ and WRITE set (mask=3). >>> Next to that the ms_async_messenger threads seem to be busy_waiting >>> looping and loading a full CPU core per thread. >>> >>> So I think I need some testing code to see what the requirements of >>> kqueue actually are, compared to what async_messenger does. >>> >>> Could be that if we want to go from (READ|WRITE) to either READ or WRITE >>> the event needs to be deleted first and than added anew. >> >> Hmm, I think I need to reread kqueue man page to figure out the problem... > > Hi, > > Perhaps we should take the Ceph-dev list off the CC, since I doubt that > they would like to read all the details. Feel free to do so, and mail me > private until there is a resolution. > > Eh, perhaps it is in the manual page but it is rather vague in how it > deals with the filters internally. And how you can modify event-filters > already in the kqueue filterlist. > > You currently assume that the filters you assign are internally > summarised such that you can do > add(read) > add(write) > delete(read|write) > > Or add(read|write) > delete(write) > delete(read) > > What I read in the man page: > EV_ADD Adds the event to the kqueue. Re-adding an existing event > will modify the parameters of the original event, and not > result in a duplicate entry. Adding an event automatically > enables it, unless overridden by the EV_DISABLE flag. > > EV_DELETE Removes the event from the kqueue. Events which are > attached to file descriptors are automatically deleted on > the last close of the descriptor. > > So I would read that as that you need to do any changes to the > filter-flags doing EV_ADD. And only removing the full event should be > done with EV_DELETE. > And an event is identified by its "ident" only: > ident Value used to identify this event. The exact interpretation > is determined by the attached filter, but often is a file > descriptor. > So any of the other settings are not used in determining which event to > add/modify/delete. > > Now the problem lies in the "modify" word in EV_ADD. I woul;d read that > as overwrite. Your code assumes that flags are merged when doing ADD or > DELETE. > > I think the code should use ADD with the settings/mask that needs to be > there, and only DELETE when the required filtermask is actually 0. > I just remember four years ago(I'm a bsd fan that time) I have written kqueue codes(https://github.com/yuyuyu101/twemproxy/commit/68d04283ba8d58142e544868842307d66d8f4600#diff-ea1f3d7f51a1def56821fefd14eb8560R193). EV_SET(&event, fd, EVFILT_READ|EV_CLEAR, EV_ADD, 0, 0, data); I think we can follow this.... > Does this make sense? > > --WjW > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
