Re: [PATCH RESEND v4] fs/epoll: Remove unnecessary wakeups of nested epoll that in ET mode

[Roman Penyaev <rpenyaev@xxxxxxx>]

On 2019-10-07 20:43, Jason Baron wrote:

[...]

> > But what if to make this wakeup explicit if we have more events to 
> > process?
> > (nothing is tested, just a guess)
> >
> > @@ -255,6 +255,7 @@ struct ep_pqueue {
> >  struct ep_send_events_data {
> >         int maxevents;
> >         struct epoll_event __user *events;
> > +       bool have_more;
> >         int res;
> >  };
> > @@ -1783,14 +1768,17 @@ static __poll_t ep_send_events_proc(struct
> > eventpoll *ep, struct list_head *head
> >  }
> >
> >  static int ep_send_events(struct eventpoll *ep,
> > -                         struct epoll_event __user *events, int 
> > maxevents)
> > +                         struct epoll_event __user *events, int 
> > maxevents,
> > +                         bool *have_more)
> >  {
> > -       struct ep_send_events_data esed;
> > -
> > -       esed.maxevents = maxevents;
> > -       esed.events = events;
> > +       struct ep_send_events_data esed = {
> > +               .maxevents = maxevents,
> > +               .events = events,
> > +       };
> >
> >         ep_scan_ready_list(ep, ep_send_events_proc, &esed, 0, false);
> > +       *have_more = esed.have_more;
> > +
> >         return esed.res;
> >  }
> >
> > @@ -1827,7 +1815,7 @@ static int ep_poll(struct eventpoll *ep, struct
> > epoll_event __user *events,
> >  {
> >         int res = 0, eavail, timed_out = 0;
> >         u64 slack = 0;
> > -       bool waiter = false;
> > +       bool waiter = false, have_more;
> >         wait_queue_entry_t wait;
> >         ktime_t expires, *to = NULL;
> >
> > @@ -1927,7 +1915,8 @@ static int ep_poll(struct eventpoll *ep, struct
> > epoll_event __user *events,
> >          * more luck.
> >          */
> >         if (!res && eavail &&
> > -           !(res = ep_send_events(ep, events, maxevents)) && 
> > !timed_out)
> > +           !(res = ep_send_events(ep, events, maxevents, &have_more)) 
> > &&
> > +           !timed_out)
> >                 goto fetch_events;
> >
> >         if (waiter) {
> > @@ -1935,6 +1924,12 @@ static int ep_poll(struct eventpoll *ep, struct
> > epoll_event __user *events,
> >                 __remove_wait_queue(&ep->wq, &wait);
> >                 spin_unlock_irq(&ep->wq.lock);
> >         }
> > +       /*
> > +        * We were not able to process all the events, so immediately
> > +        * wakeup other waiter.
> > +        */
> > +       if (res > 0 && have_more && waitqueue_active(&ep->wq))
> > +               wake_up(&ep->wq);
> >
> >         return res;
> >  }

[...]

> And I think the above change can go in separately (if we decide we want 
> it).

Hi Jason,

I did measurements using Eric's test http://yhbt.net/eponeshotmt.c
(8 writers, 8 waiters;  1 writer, 8 waiters) and tested the impact
of outrunning wakeup: I do not see any difference. Since write events
are constantly coming, next waiter will be woken up anyway by the
following write event.  In order to have some perf gain probably writes
should happen with some interval: produce bunch of events, sleep,
produce bunch of events, sleep, etc, which seems can bring something
only if writer is accidentally synchronized with waiters. Not a clean
way of perf improvement.

--
Roman