lkml.org 
[lkml]   [2009]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH -v5][RFC]: mutex: implement adaptive spinning


On Wed, 7 Jan 2009, Linus Torvalds wrote:
>
> We've needed that before (and yes, we've simply mis-used __get_user() on
> x86 before rather than add it).

Ahh, yeah, we have a really broken form of this in probe_kernel_address(),
but it's disgustingly slow. And it actually does a whole lot more, since
it is designed for addresses that we can't trust.

The thing about "thread->cpu" is that we can actually _trust_ the address,
so we know it's not a user address or anything like that. So we don't need
the whole pagefault_disable/enable around it. At least the x86 page fault
handler knows that it absolutely must not take the mm semaphore for these
things, for example (because it would be an insta-deadlock for the vmalloc
space and thus any mm fault handlers in filesystems that are modules).

So we would be better off without that, but we could certainly also
improve probe_kernel_address() if we had a better model. IOW, we *could*
do something like

#define get_kernel_mode(val, addr) __get_user(val, addr)

on the simple platforms, and then in the generic <linux/uaccess.h> code we
can just do

#ifndef get_kernel_mode
#define get_kernel_mode(val, addr) ({ \
long _err; \
mm_segment_t old_fs = get_fs(); \
set_fs(KERNEL_DS); \
_err = __get_user(val, addr); \
set_fs(old_fs); \
_err; })
#endif

and then probe_kernel_address becomes

#define probe_kernel_address(addr, val) ({ \
long _err; \
pagefault_disable(); \
_err = get_kernel_mode(val, addr); \
pagefault_enable(); \
_err; })

which leaves all architectures with the trivial option to just define
their own 'get_kernel_mode()' thing that _often_ is exactly the same as
__get_user().

Hmm? Anybody want to test it?

Linus


\
 
 \ /
  Last update: 2009-01-07 22:43    [W:0.281 / U:1.992 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site