From: Eric Wong <normalperson@yhbt.net>
To: "Arve Hjønnevåg" <arve@android.com>
Cc: NeilBrown <neilb@suse.de>, "Rafael J. Wysocki" <rjw@sisk.pl>,
linux-kernel@vger.kernel.org,
Davide Libenzi <davidel@xmailserver.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>
Subject: epoll: possible bug from wakeup_source activation
Date: Thu, 7 Mar 2013 11:26:39 +0000 [thread overview]
Message-ID: <20130307112639.GA25130@dcvr.yhbt.net> (raw)
Hi Arve, looking at commit 4d7e30d98939a0340022ccd49325a3d70f7e0238
(epoll: Add a flag, EPOLLWAKEUP, to prevent suspend ...)
I think the reason for using ep->ws instead of epi->ws in the unlikely
ovflist case applies to the likely rdllist case, too. Since epi->ws is
only protected by ep->mtx, it can also be deactivated while inside
ep_poll_callback.
So something like the following patch might be necessary
(shown here with extra context):
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -968,39 +968,45 @@ static int ep_poll_callback(wait_queue_t *wait, unsigned mode, int sync, void *k
if (unlikely(ep->ovflist != EP_UNACTIVE_PTR)) {
if (epi->next == EP_UNACTIVE_PTR) {
epi->next = ep->ovflist;
ep->ovflist = epi;
if (epi->ws) {
/*
* Activate ep->ws since epi->ws may get
* deactivated at any time.
*/
__pm_stay_awake(ep->ws);
}
}
goto out_unlock;
}
/* If this file is already in the ready list we exit soon */
if (!ep_is_linked(&epi->rdllink)) {
list_add_tail(&epi->rdllink, &ep->rdllist);
- __pm_stay_awake(epi->ws);
+ if (epi->ws) {
+ /*
+ * Activate ep->ws since epi->ws may get
+ * deactivated at any time.
+ */
+ __pm_stay_awake(ep->ws);
+ }
}
-----------------------------------------------------------------------
However, I think my proposed patch will also cause breakage with the
way epi->ws is handled in ep_send_events_proc:
/*
* Activate ep->ws before deactivating epi->ws to prevent
* triggering auto-suspend here (in case we reactive epi->ws
* below).
*
* This could be rearranged to delay the deactivation of epi->ws
* instead, but then epi->ws would temporarily be out of sync
* with ep_is_linked().
*/
if (epi->ws && epi->ws->active)
__pm_stay_awake(ep->ws);
__pm_relax(epi->ws);
list_del_init(&epi->rdllink);
I'm not sure, but maybe only using ep->ws is the easiest way to go.
next reply other threads:[~2013-03-07 11:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-07 11:26 Eric Wong [this message]
2013-03-08 1:30 ` Eric Wong
2013-03-08 4:56 ` Arve Hjønnevåg
2013-03-08 20:49 ` Eric Wong
2013-03-09 4:09 ` Arve Hjønnevåg
2013-03-09 7:10 ` Eric Wong
2013-03-10 1:11 ` Eric Wong
2013-03-10 4:59 ` Eric Dumazet
2013-03-10 11:50 ` [PATCH] epoll: use RCU protect wakeup_source in epitem Eric Wong
2013-03-14 3:09 ` [PATCH mm] epoll: lock ep->mtx in ep_free to silence lockdep Eric Wong
2013-03-11 23:37 ` epoll: possible bug from wakeup_source activation Arve Hjønnevåg
2013-03-12 0:17 ` Eric Wong
2013-03-12 0:29 ` Arve Hjønnevåg
2013-03-12 0:44 ` 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=20130307112639.GA25130@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=neilb@suse.de \
--cc=rjw@sisk.pl \
--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
all inboxes | Powered by JetHome®