public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Wong <normalperson@yhbt.net>
To: "Arve Hjønnevåg" <arve@android.com>
Cc: linux-kernel@vger.kernel.org,
	Davide Libenzi <davidel@xmailserver.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [RFC v3 1/2] epoll: avoid spinlock contention with wfcqueue
Date: Fri, 22 Mar 2013 03:24:10 +0000	[thread overview]
Message-ID: <20130322032410.GA19377@dcvr.yhbt.net> (raw)
In-Reply-To: <CAMP5XgfKfuF4Lo1PVoqBnjynB6COWVhVcqzKV3PAic7HLP7FaQ@mail.gmail.com>

Arve Hjønnevåg <arve@android.com> wrote:
> On Thu, Mar 21, 2013 at 4:52 AM, Eric Wong <normalperson@yhbt.net> wrote:
> > Changes since v2:
> > * epi->state is no longer atomic, we only cmpxchg in ep_poll_callback
> >   now and rely on implicit barriers in other places for reading.
> > * intermediate EP_STATE_DEQUEUE removed, this (with xchg) caused too
> >   much overhead in the ep_send_events loop and could not eliminate
> >   starvation dangers from improper EPOLLET usage (the original code
> >   had this problem, too, the window is just a few cycles larger, now).
> > * minor code cleanups

> >                 /*
> >                  * Activate ep->ws before deactivating epi->ws to prevent
> 
> Does anything deactivate ep->ws now?

Oops, I left that out when I killed ep_scan_ready_list.
But I think we need a different approach to wakeup sources in
this series...

> > +               /*
> > +                * reset item state for EPOLLONESHOT and EPOLLET
> > +                * no barrier here, rely on ep->mtx release for write barrier
> > +                */
> 
> What happens if ep_poll_callback runs before you set epi->state below?
> It used to queue on ep->ovflist and call __pm_stay_awake on ep->ws,
> but now it does not appear to do anything.
> 
> > +               epi->state = EP_STATE_IDLE;
> >         }
> >
> >         return eventcnt;
> >  }
> >

With EPOLLET and improper usage (not hitting EAGAIN), the event now
has a larger window to be lost (as mentioned in my changelog).

As far as correct __pm_stay_awake/__pm_relax handling, perhaps adding
an atomic counter to struct eventpoll (or each epitem) will work?

If we go with atomic counter in struct eventpoll, is per-epitem
wakeup_source still necessary?  We have space in epitem now, but
maybe one day we will might need it.

Thanks for looking at this patch.

Btw, I'm curious; which applications use EPOLLWAKEUP?

My epoll work is focused on network servers with thousands of clients,
and I don't think any of them use (or have use for) EPOLLWAKEUP.
But I will keep EPOLLWAKEUP users in mind when working on epoll :)

  reply	other threads:[~2013-03-22  3:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-21 11:52 [RFC v3 1/2] epoll: avoid spinlock contention with wfcqueue Eric Wong
2013-03-21 11:54 ` [RFC v3 2/2] epoll: use a local wfcq functions for Level Trigger Eric Wong
2013-03-21 12:01 ` [RFC v3 1/2] epoll: avoid spinlock contention with wfcqueue Eric Wong
2013-03-22  1:39 ` Arve Hjønnevåg
2013-03-22  3:24   ` Eric Wong [this message]
2013-03-22  4:07     ` Arve Hjønnevåg
2013-03-22 10:31       ` Eric Wong
2013-03-22 19:24         ` Eric Wong
2013-03-22 22:27           ` Arve Hjønnevåg
2013-03-22 22:18         ` Arve Hjønnevåg
2013-03-22 22:54           ` Eric Wong
2013-03-23 10:16             ` Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130322032410.GA19377@dcvr.yhbt.net \
    --to=normalperson@yhbt.net \
    --cc=akpm@linux-foundation.org \
    --cc=arve@android.com \
    --cc=davidel@xmailserver.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox