From: Davide Libenzi <davidel@xmailserver.org>
To: Shawn Bohrer <shawn.bohrer@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Jack Stone <jwjstone@fastmail.fm>,
Viresh Kumar <viresh.kumar@st.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk
Subject: Re: [PATCH 3/3] epoll: remove unnecessary test of ep->ovflist for available events
Date: Sat, 15 Jan 2011 11:03:59 -0800 (PST) [thread overview]
Message-ID: <alpine.DEB.2.00.1101150938090.6884@davide-lnx1> (raw)
In-Reply-To: <1295110837-3061-3-git-send-email-shawn.bohrer@gmail.com>
On Sat, 15 Jan 2011, Shawn Bohrer wrote:
> The additional test for ep->ovflist != EP_UNACTIVE_PTR to signify
> available events was added in 5071f97ec6d74f006072de0ce89b67c8792fe5a1
> but doesn't appear to do anything. Either this is a bug or the check
> isn't needed.
>
> If the ep->ovflist is not EP_UNACTIVE_PTR then ep_send_events() calls
> ep_scan_ready_list() which sets ep->ovflist = NULL thus loosing any
> events which may have been stored there.
>
> Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>
NACK. Not only NACK, but hell NACK.
The epoll_wait() might hit right after the delivery of the current events
ended in/right-after:
error = (*sproc)(ep, &txlist, priv);
So, if there are overflowed events, a following ep_send_events() can go
fetch them, because ep_scan_ready_list() will go drop them back in the
ready list (right after the line above).
Events in the overflow list are no different from the ones in the ready
list, and removing such test will make you, either return with no events
when they are really there, or take another unnecessary spin lock/unlock
trip.
On the contrary, a missed optimization is applying the same rule even
above, instead of the bare list_empty(). Will send a patch to Andrew.
And no, it is not a bug, because ep_scan_ready_list() is protected by a
mutex.
- Davide
prev parent reply other threads:[~2011-01-15 19:04 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-14 11:52 [PATCH resend] fs/eventpoll.c: fix compilation warning Viresh Kumar
2011-01-14 13:07 ` Jack Stone
2011-01-14 14:48 ` Shawn Bohrer
2011-01-14 15:21 ` Davide Libenzi
2011-01-15 0:05 ` Andrew Morton
2011-01-15 11:10 ` Jack Stone
2011-01-15 16:20 ` Shawn Bohrer
2011-01-15 17:00 ` [PATCH 1/3] epoll: initialize slack for negative timeout values Shawn Bohrer
2011-01-15 19:06 ` Davide Libenzi
2011-03-18 7:38 ` Mike Frysinger
2011-01-15 17:00 ` [PATCH 2/3] epoll: short circuit the timeout==0 case Shawn Bohrer
2011-01-15 17:00 ` [PATCH 3/3] epoll: remove unnecessary test of ep->ovflist for available events Shawn Bohrer
2011-01-15 19:03 ` Davide Libenzi [this message]
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=alpine.DEB.2.00.1101150938090.6884@davide-lnx1 \
--to=davidel@xmailserver.org \
--cc=akpm@linux-foundation.org \
--cc=jwjstone@fastmail.fm \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=shawn.bohrer@gmail.com \
--cc=viresh.kumar@st.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
all inboxes | Powered by JetHome®