Messages in this thread | | | Date | Thu, 27 Jan 2011 16:17:22 -0800 (PST) | From | Davide Libenzi <> | Subject | Re: sys_epoll_wait high CPU load in 2.6.37 |
| |
On Wed, 26 Jan 2011, Eric Dumazet wrote:
> Le mercredi 26 janvier 2011 à 19:18 +0100, Eric Dumazet a écrit : > > Le mercredi 26 janvier 2011 à 10:06 -0800, Davide Libenzi a écrit : > > > > > Eric, if you look at fs/select.c (~line 925), poll does exactly the same > > > thing as epoll do. > > > It too, ignores the eventual return value of poll_select_set_timeout(), so > > > maybe a little bit more optimized ktime_get_ts+timespec_add_ms could make > > > sense. > > > > > > > OK, I'll post a V3 ;) > > Well in the poll() case we handle a zero timeout, not in epoll(). > > So the helper function cannot be shared and can be static to epoll. > > > [PATCH v3] epoll: epoll_wait() should not use timespec_add_ns() > > commit 95aac7b1cd224f (epoll: make epoll_wait() use the hrtimer range > feature) added a performance regression because it uses > timespec_add_ns() with potential very large 'ns' values. > > Reported-by: Simon Kirby <sim@hostway.ca> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > CC: Shawn Bohrer <shawn.bohrer@gmail.com> > CC: Davide Libenzi <davidel@xmailserver.org> > CC: Thomas Gleixner <tglx@linutronix.de> > CC: Andrew Morton <akpm@linux-foundation.org> > --- > fs/eventpoll.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/fs/eventpoll.c b/fs/eventpoll.c > index cc8a9b7..d517aa3 100644 > --- a/fs/eventpoll.c > +++ b/fs/eventpoll.c > @@ -1114,6 +1114,17 @@ static int ep_send_events(struct eventpoll *ep, > return ep_scan_ready_list(ep, ep_send_events_proc, &esed); > } > > +static inline struct timespec epoll_set_mstimeout(long ms) > +{ > + struct timespec now, ts = { > + .tv_sec = ms / MSEC_PER_SEC, > + .tv_nsec = NSEC_PER_MSEC * (ms % MSEC_PER_SEC), > + }; > + > + ktime_get_ts(&now); > + return timespec_add_safe(now, ts); > +} > + > static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events, > int maxevents, long timeout) > { > @@ -1121,12 +1132,11 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events, > unsigned long flags; > long slack; > wait_queue_t wait; > - struct timespec end_time; > ktime_t expires, *to = NULL; > > if (timeout > 0) { > - ktime_get_ts(&end_time); > - timespec_add_ns(&end_time, (u64)timeout * NSEC_PER_MSEC); > + struct timespec end_time = epoll_set_mstimeout(timeout); > + > slack = select_estimate_accuracy(&end_time); > to = &expires; > *to = timespec_to_ktime(end_time);
Looks OK for me. The epoll_ prefix fights with the ep_ standard on the original file, but I will post a one-liner for it, given Andrew already got this.
- Davide
| |