lkml.org 
[lkml]   [2010]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] futex: remove rw parameter from get_futex_key()
Date
> 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]
\
 
 \ /
  Last update: 2010-01-06 03:29    [W:0.054 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site