> > Assume we revert this commit, the current logic looks good to me. We > either get a 'fd' or a 'wevent' (can't get both). If we get a 'fd', we > create a wevent and associate that 'fd' with 'wevent' via > WSAEventSelect(). If we only get wevent, it will simply go to > WaitForMultipleObjects(). I see now what you mean about a problem in find_poll_node(). I think separating it out like you mention won't work though. We need to check if 'fd' is specified, in which case we 'if' on 'fd'. If wevent is specified, 'if' on 'wevent'.
> > > >> >> Alin. >> >> >>> -----Mesaj original----- >>> De la: Gurucharan Shetty [mailto:[email protected]] >>> Trimis: Wednesday, September 30, 2015 5:24 AM >>> Către: Alin Serdean <[email protected]> >>> Cc: Ben Pfaff <[email protected]>; Ilya Maximets <[email protected]>; >>> dev <[email protected]>; Dyasly Sergey <[email protected]> >>> Subiect: Re: [ovs-dev] [PATCH] poll-loop: fix assertion in poll_create_node >>> >>> I think Ben's patch would have fixed the problem had pollfd.fd actually >>> held a >>> negative value. Documentation in msdn says that it can, but when I step >>> through the debugger, I see that a -1 set to that shows up us a large >>> positive >>> integer. For e.g., if I apply this hypothetical patch, I don't see test >>> failures. >>> >>> diff --git a/lib/poll-loop.c b/lib/poll-loop.c index 36eb5ac..850eeac 100644 >>> --- a/lib/poll-loop.c >>> +++ b/lib/poll-loop.c >>> @@ -297,7 +297,7 @@ free_poll_nodes(struct poll_loop *loop) >>> HMAP_FOR_EACH_SAFE (node, next, hmap_node, &loop->poll_nodes) { >>> hmap_remove(&loop->poll_nodes, &node->hmap_node); #ifdef >>> _WIN32 >>> - if (node->wevent && node->pollfd.fd) { >>> + if (node->wevent && node->pollfd.fd <= 4400000) { >>> WSAEventSelect(node->pollfd.fd, NULL, 0); >>> CloseHandle(node->wevent); >>> } >>> @@ -341,7 +341,7 @@ poll_block(void) >>> pollfds[i] = node->pollfd; >>> #ifdef _WIN32 >>> wevents[i] = node->wevent; >>> - if (node->pollfd.fd && node->wevent) { >>> + if (node->pollfd.fd <= 4400000 && node->wevent) { >>> short int wsa_events = 0; >>> if (node->pollfd.events & POLLIN) { >>> wsa_events |= FD_READ | FD_ACCEPT | FD_CLOSE; >>> >>> >>> I actually cannot understand what was the bug that created the original >>> commit in question. I cannot think of a place in OVS where we send fd as >>> zero to create_poll_node(). >>> >>> Ilya, >>> Can you explain the original trigger that caused you to submit this patch? >>> I >>> would rather revert this unless I understand the reason. >>> >>> >>> On Tue, Sep 29, 2015 at 5:07 PM, Gurucharan Shetty <[email protected]> >>> wrote: >>> > hash_2words takes unsigned int. I guess that is the problem? >>> > >>> > On Tue, Sep 29, 2015 at 5:00 PM, Alin Serdean >>> > <[email protected]> wrote: >>> >> I think so. I think for once we should do >>> >> https://github.com/openvswitch/ovs/blob/master/lib/poll-loop.c#L119-L >>> >> 123 before >>> >> https://github.com/openvswitch/ovs/blob/master/lib/poll-loop.c#L115-L >>> >> 116 >>> >> >>> >> I am compiling under debug and attaching a debugger to it. >>> >> >>> >> Alin. >>> >> >>> >>> -----Mesaj original----- >>> >>> De la: Gurucharan Shetty [mailto:[email protected]] >>> >>> Trimis: Wednesday, September 30, 2015 2:34 AM >>> >>> Către: Alin Serdean <[email protected]> >>> >>> Cc: Ben Pfaff <[email protected]>; Ilya Maximets >>> >>> <[email protected]>; dev <[email protected]>; Dyasly >>> Sergey >>> >>> <[email protected]> >>> >>> Subiect: Re: [ovs-dev] [PATCH] poll-loop: fix assertion in >>> >>> poll_create_node >>> >>> >>> >>> Alin, >>> >>> Reverting this commit on tip of master does not give me any unit >>> >>> test failures. When you say there is more to it, do you mean that >>> >>> this commit actually exposed some other bug? _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
