Messages in this thread | | | From | KOSAKI Motohiro <> | Subject | Re: [PATCH v2] futex: remove rw parameter from get_futex_key() | Date | Wed, 6 Jan 2010 11:27:08 +0900 (JST) |
| |
> Hugh Dickins wrote: > > On Tue, 5 Jan 2010, KOSAKI Motohiro wrote: > > >> From c3e2dfdff84b9b720e646fd6dd3c38fff293c7e6 Mon Sep 17 00:00:00 2001 > >> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > >> Date: Tue, 5 Jan 2010 11:33:00 +0900 > >> Subject: [PATCH] futex: remove rw parameter from get_futex_key() > >> > >> Currently, futex have two problem. > >> > >> A) current futex doesn't handle private file mappings properly. > >> > >> get_futex_key() use PageAnon() to distinguish file and anon. it can > >> makes following bad scenario. > >> > >> 1) thread-A call futex(private-mapping, FUTEX_WAIT). it makes to > >> sleep on file mapping object. > >> 2) thread-B write a variable and it makes cow. > >> 3) thread-B call futex(private-mapping, FUTEX_WAKE). it wake up > >> sleeped thread on the anonymous page. (but it's nothing) > >> > > Excellent test case, thank you! Would you consider preparing a patch to > futextest? > > http://git.kernel.org/?p=linux/kernel/git/dvhart/futextest.git;a=summary
Patch attached. you can feel free any modify such file. thanks.
> I did some experimentation here and found that: > > o The test works if the *_PRIVATE op codes are used. > This is because the futex keys are generated using only the virtual > address of the page, which doesn't change on a COW. > o If the waiter writes to the val first, it works. > This forces the COW before the waiter generates it's futex key.
True.
> So the waiter's key is generated based on the page cache page address > for shared futexes when the value hasn't been written to prior to wait. > > The only scenario where I could think of wanting this behavior is if > another process were to try and wake the waiter via the same file backed > page. However, as I understand it, the re-use of the same page for > unwritten-to private pages is an optimization and can't be relied upon. > So this scenario is out. Another would be to use the futex as a very > simple wait queue where the value is never changed. In this case > however, the implementation is racy as the value check is effectively > negated, so this use case is also out. > > As such, I see no reason not to always use VERIFY_WRITE and force a COW > prior to generating the futex_key for shared futexes. It is not > necessary for private futexes however as they use only the virtual address. > > I am not sure on whether or not it makes sense to avoid the VERIFY_WRITE > on the private futexes. Could be it is just more code for negligible > benefit. Thoughts?
I think it's no problem. because,
as performance view: access_ok() of almost arch (included x86) ignore VERIFY_WRITE. then, this change doesn't cause performance loss of course. (current futex mainly handle ro-mapping problem by fault_in_user_writeable)
as consistency view: This patch have better consistency without FUTEX_PRIVATE_FLAG case.
as usability view: Nobody want to use ro-mappings for private futex. it's obviously meaningless and useless.
[unhandled content-type:application/octet-stream] | |