Re: Question about PRIVATE_FUTEX

[Minchan Kim]

On Fri, Mar 27, 2009 at 8:14 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Fri, 2009-03-27 at 19:56 +0900, Minchan Kim wrote:
>
>> >> Then, get_futex_value_locked calls __cpy_from_user_inatomic with
>> >> pagefault_disable.
>> >>
>> >> Who make sure the user page is mapped at app's page table ?
>> >
>> > Nobody, all uses of get_futex_value_locked() have to deal with it
>> > returning -EFAULT.
>>
>> Does It mean that __copy_from_user_inatomic in get_futex_value_locked
>> would be failed rather than sleep?
>
> Correct.
>
>> In fact, I don't make sure _copy_from_user_inatomic function's meaning.
>> As far as I understand, It never sleep. It just can be failed in case
>> of user page isn't mapped. Is right ?
>
> Correct.
>
>> Otherwise, it can be scheduled with pagefault_disable which increments
>> preempt_count. It is a atomic bug.
>> If my assume is right, it can be failed rather than sleep.
>> At this case, other architecture implements __copy_from_user_inatomic
>> with __copy_from_user which can be scheduled. It also can be bug.
>>
>> Hmm, Now I am confusing.
>
> Confused I guess ;-)
> The trick is in the in_atomic() check in the pagefault handler and the
> fixup section of the copy routines.

Whew~,  There was good hidden trick.
I will dive into this assembly.
I always thanks for your kindness. :)

> #define __copy_user(to, from, size) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> do { Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Âint __d0, __d1, __d2; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â__asm__ __volatile__( Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â cmp Â$7,%0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â jbe Â1f\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â" Â Â Â movl %1,%0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â negl %0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â" Â Â Â andl $7,%0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â subl %0,%3\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â"4: Â Â rep; movsb\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â movl %3,%0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â shrl $2,%0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â andl $3,%3\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â .align 2,0x90\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â"0: Â Â rep; movsl\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â movl %3,%0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â"1: Â Â rep; movsb\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â"2:\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â".section .fixup,\"ax\"\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â"5: Â Â addl %3,%0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â jmp 2b\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â"3: Â Â lea 0(%3,%0,4),%0\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â" Â Â Â jmp 2b\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â".previous\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â".section __ex_table,\"a\"\n" Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â .align 4\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â" Â Â Â .long 4b,5b\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â" Â Â Â .long 0b,3b\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â" Â Â Â .long 1b,2b\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â".previous" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â Â Â Â Â Â Â: "=&c"(size), "=&D" (__d0), "=&S" (__d1), "=r"(__d2) Â \
> Â Â Â Â Â Â Â Â: "3"(size), "0"(size), "1"(to), "2"(from) Â Â Â Â Â Â Â\
> Â Â Â Â Â Â Â Â: "memory"); Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> } while (0)
>
> see that __ex_table section, it tells the fault handler where to
> continue in case of an atomic fault.
>
>> > Most of this is legacy btw, from when futex ops were done under the
>> > mmap_sem. Back then we couldn't fault because that would cause mmap_sem
>> > recursion. Howver, now that we don't hold mmap_sem anymore we could use
>> > a faulting user access like get_user().
>> > Darren has been working on patches to clean that up, some of those are
>> > already merged in the -tip tree.
>>
>> Thanks for good information.
>> It will be very desirable way to enhance kernel performance.
>
> I doubt it'll make a measurable difference, if you need to fault
> performance sucks anyway. If you don't, the current code is just as
> fast.
>



-- 
Kinds regards,
Minchan Kim
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i