On Tue, Jul 20, 2021 at 08:19:35AM +0100, Anton Ivanov wrote: > > On 19/07/2021 21:52, Ben Pfaff wrote: > > On Fri, Jul 16, 2021 at 02:42:32PM +0100, [email protected] > > wrote: > > > From: Anton Ivanov <[email protected]> > > > > > > If we are not obtaining any useful information out of the poll(), > > > such as is a fd busy or not, we do not need to do a poll() if > > > an immediate_wake() has been requested. > > > > > > This cuts out all the pollfd hash additions, forming the poll > > > arguments and the actual poll() after a call to > > > poll_immediate_wake() > > > > > > Signed-off-by: Anton Ivanov <[email protected]> > > Thanks for v3. > > > > I believe that the existing code here properly handles the pathological > > case where the current time is less than 0. This is a case that I have > > seen happen on real systems that have a real-time clock with a bad > > current time and do not have proper support for a monotonic clock. I > > believe that your new code does not properly handle this case, because > > it treats all times less than 0 as immediate. I think that you can fix > > it by comparing against LLONG_MIN rather than zero. > In that case, we should set poll_immediate_wake to be LLONG_MIN as well, > right?
It does that, even though it's a bit indirect, since poll_immediate_wake_at() passes 0 to poll_timer_wait_at(), which translates 0 to LLONG_MIN. > > In poll-loop, I would move > > COVERAGE_INC(poll_zero_timeout); > > inside the new statement that is already conditional on timeout_when == > > LLONG_MIN. > > > > I don't like the new assumption in time_poll() that n_pollfds == 0 means > > we're waking immediately. > > We can do everything on timeout. That is more consistent. Thanks. > As far as assumptions go, it is equivalent on Unixes, because the > number of fds is always > 1. This is because you always get 1 fd from > the signal pipe. That's true if poll_block() is the only caller of time_poll(), which is currently true, as long as we continue to deal with signals this way. But we don't document or enforce that assumption and I'd rather not rely on it. Thanks, Ben. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
