linux-kernel - Re: [PATCH v2] epoll: Support for disabling items, and a self-test app.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <508044D7.7070005@mit.edu>
Date:	Thu, 18 Oct 2012 11:05:11 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	mtk.manpages@...il.com, "Paton J. Lewis" <palewis@...be.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Jason Baron <jbaron@...hat.com>, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, Paul Holland <pholland@...be.com>,
	Davide Libenzi <davidel@...ilserver.org>,
	libc-alpha@...rceware.org, Linux API <linux-api@...r.kernel.org>,
	paulmck@...ux.vnet.ibm.com
Subject: Re: [PATCH v2] epoll: Support for disabling items, and a self-test
 app.

[cc Paul McKenney, who is probably the leading expert on these things]

On 10/17/2012 04:30 PM, Andrew Morton wrote:
> On Tue, 16 Oct 2012 17:12:57 +0200
> "Michael Kerrisk (man-pages)" <mtk.manpages@...il.com> wrote:
> 
>> On Thu, Aug 23, 2012 at 11:15 PM, Paton J. Lewis <palewis@...be.com> wrote:
>>> From: "Paton J. Lewis" <palewis@...be.com>
>>>
>>> Enhanced epoll_ctl to support EPOLL_CTL_DISABLE, which disables an epoll item.
>>> If epoll_ctl doesn't return -EBUSY in this case, it is then safe to delete the
>>> epoll item in a multi-threaded environment. Also added a new test_epoll self-
>>> test app to both demonstrate the need for this feature and test it.
>>
>> (There's a lot of background missing from this version of the patch
>> that was included in the previous version
>> [http://thread.gmane.org/gmane.linux.kernel/1311457]. It helps to
>> include the full rationale with a revised patch--best not to assume
>> that someone has the context of past mails when reading a revised
>> patch.)
>>
>> I've taken a look at this patch as it currently stands in 3.7-rc1, and
>> done a bit of testing. (By the way, the test program
>> tools/testing/selftests/epoll/test_epoll.c does not compile...)
> 
> Thanks for this.  You raise significant issues.  If we can't get these
> fully resolved over the next month or so, we should revert the patch so
> this new API doesn't get released in 3.7.  I have queued a patch to do
> this and shall maintain it while I watch developments...

I can't shake the feeling that EPOLL_CTL_DISABLE is solving a
non-problem, or, more precisely, that there should be a perfectly good
userspace solution with no kernel changes.

Unless something is rather buggy in kernel land (and I don't think it
is), once EPOLL_CTL_DEL has returned, no call to epoll_wait that starts
*after* EPOLL_CTL_DEL finishes will return that object.  This suggests
an RCU-like approach: once EPOLL_CTL_DEL has returned and every thread
has returned from an epoll_wait call that started after the
EPOLL_CTL_DEL returns, then the data structure can be safely freed.

In pseudocode:

delete(fd, pdata) {
  pdata->dead = true;
  EPOLL_CTL_DEL(fd);
  rcu_call(delete pdata);
}

wait() {
  epoll_wait;
  for each event pdata {
    if (pdata->gone) continue;
    process the event;
  }

  rcu_this_is_a_grace_period();
}

Of course, these are not normal grace periods and would need to be
tracked separately.  (The optimal data structure to do this without
killing scalability is not obvious.  urcu presumably implements such a
thing.)

Am I right?


--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/