> Sure, it probably is a non issue, but I'm afraid that non issues of today
> might become issues of tomorrow. Where does it say that we can never put a
> task struct in a movable zone.
Task structs are not movable, so they by definition do not belong
in movable zones.
> memory local to it, and
On Thu, 8 Jan 2009, Andi Kleen wrote:
> > What about memory hotplug as Ingo mentioned?
> >
> > Should that be:
> >
> > #if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
>
> We expect memory hotunplug to only really work in movable zones
> (all others should at least have on
On Wed, 2009-01-07 at 15:32 -0800, Linus Torvalds wrote:
>
> On Wed, 7 Jan 2009, Steven Rostedt wrote:
> >
> > What would be interesting is various benchmarks against all three.
> >
> > 1) no mutex spinning.
> > 2) get_task_struct() implementation.
> > 3) spin_or_sched implementation.
>
> One o
> What about memory hotplug as Ingo mentioned?
>
> Should that be:
>
> #if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
We expect memory hotunplug to only really work in movable zones
(all others should at least have one kernel object somewhere that prevents
unplug) and you
Steven Rostedt wrote:
> On Wed, 7 Jan 2009, Gregory Haskins wrote:
>
>> In my defense, the -rt versions of the patches guarantee this is ok
>> based on a little hack:
>>
>
> The -rt versions worry about much more than what the mutex code in
> mainline does. Linus is correct in his arguments
On Wed, 7 Jan 2009, Gregory Haskins wrote:
>
> In my defense, the -rt versions of the patches guarantee this is ok
> based on a little hack:
The -rt versions worry about much more than what the mutex code in
mainline does. Linus is correct in his arguments. The adaptive mutex (as
suppose to wha
[resend: restore CC list]
>>> Linus Torvalds 01/07/09 6:33 PM >>>
>On Wed, 7 Jan 2009, Steven Rostedt wrote:
>>
>> What would be interesting is various benchmarks against all three.
>>
>> 1) no mutex spinning.
>> 2) get_task_struct() implementation.
>> 3) spin_or_sched implementation.
>
>One of
[resend: i fat fingered the reply-to-all for a few messages]
>>> Linus Torvalds 01/07/09 6:20 PM >>>
>On Wed, 7 Jan 2009, Linus Torvalds wrote:
>> >
>> > "Is get_task_struct() really that bad?"
>>
>> Yes. It's an atomic access (two, in fact, since you need to release it
>> too), which is a huge
On Wed, 7 Jan 2009 21:33:31 -0500 (EST)
Steven Rostedt wrote:
>
> On Wed, 7 Jan 2009, Linus Torvalds wrote:
> > > Should that be:
> > >
> > > #if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
> >
> > Well, probably CONFIG_MEMORY_HOTREMOVE, no? And I'd actually suggest that
On Wed, 7 Jan 2009, Linus Torvalds wrote:
> > Should that be:
> >
> > #if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
>
> Well, probably CONFIG_MEMORY_HOTREMOVE, no? And I'd actually suggest that
> unplugging should have a stop-machine if it doesn't already, just because
On Wed, 7 Jan 2009 15:57:06 -0800 (PST)
Linus Torvalds wrote:
>
>
> On Wed, 7 Jan 2009, Steven Rostedt wrote:
> >
> > Should that be:
> >
> > #if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
>
> Well, probably CONFIG_MEMORY_HOTREMOVE, no? And I'd actually suggest that
>
On Wed, 7 Jan 2009, Steven Rostedt wrote:
>
> Should that be:
>
> #if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_MEMORY_HOTPLUG)
Well, probably CONFIG_MEMORY_HOTREMOVE, no? And I'd actually suggest that
unplugging should have a stop-machine if it doesn't already, just because
it's suc
On Wed, 7 Jan 2009, Steven Rostedt wrote:
>
> On Wed, 7 Jan 2009, Steven Rostedt wrote:
> >
> > True. I need to keep looking at the code that is posted. In -rt, we force
> > the fast path into the slowpath as soon as another task fails to get the
> > lock. Without that, as you pointed out, t
On Wed, 7 Jan 2009, Linus Torvalds wrote:
> On Wed, 7 Jan 2009, Dave Kleikamp wrote:
> >
> > Do you need to even do that if CONFIG_DEBUG_PAGEALLOC is unset?
>
> No.
>
> > Something like:
> >
> > #ifdef CONFIG_DEBUG_PAGEALLOC
> > /*
> > * Need to access the cpu
On Wed, 7 Jan 2009, Steven Rostedt wrote:
>
> True. I need to keep looking at the code that is posted. In -rt, we force
> the fast path into the slowpath as soon as another task fails to get the
> lock. Without that, as you pointed out, the code can be racy.
I mean we force the fast unlock pat
On Wed, 7 Jan 2009, Linus Torvalds wrote:
> On Wed, 7 Jan 2009, Steven Rostedt wrote:
> >
> > What would be interesting is various benchmarks against all three.
> >
> > 1) no mutex spinning.
> > 2) get_task_struct() implementation.
> > 3) spin_or_sched implementation.
>
> One of the issues is t
On Wed, 7 Jan 2009, Steven Rostedt wrote:
>
> What would be interesting is various benchmarks against all three.
>
> 1) no mutex spinning.
> 2) get_task_struct() implementation.
> 3) spin_or_sched implementation.
One of the issues is that I cannot convince myself that (2) is even
necessarily
On Wed, Jan 07, 2009 at 05:28:12PM -0500, Gregory Haskins wrote:
> Andi Kleen wrote:
> >> I appreciate this is sample code, but using __get_user() on
> >> non-userspace pointers messes up architectures which have separate
> >> user/kernel spaces (eg the old 4G/4G split for x86-32). Do we have an
>
On Wed, 7 Jan 2009, Linus Torvalds wrote:
> >
> > "Is get_task_struct() really that bad?"
>
> Yes. It's an atomic access (two, in fact, since you need to release it
> too), which is a huge deal if we're talking about a timing-critical
> section of code.
There's another issue: you also need t
On Wed, 7 Jan 2009, Dave Kleikamp wrote:
>
> Do you need to even do that if CONFIG_DEBUG_PAGEALLOC is unset?
No.
> Something like:
>
> #ifdef CONFIG_DEBUG_PAGEALLOC
> /*
> * Need to access the cpu field knowing that
> * DEBUG_PAGEALLOC could h
On Wed, 7 Jan 2009, Gregory Haskins wrote:
>
> Can I ask a simple question in light of all this discussion?
>
> "Is get_task_struct() really that bad?"
Yes. It's an atomic access (two, in fact, since you need to release it
too), which is a huge deal if we're talking about a timing-critical
s
On Wed, 2009-01-07 at 15:51 -0700, Peter W. Morreale wrote:
> On Wed, 2009-01-07 at 23:33 +0100, Ingo Molnar wrote:
> > * Gregory Haskins wrote:
> >
> > > Can I ask a simple question in light of all this discussion?
> > >
> > > "Is get_task_struct() really that bad?"
> >
> > it dirties a cachel
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
>
> Hmm, still wouldn't the spin_on_owner() loopyness and the above need
> that need_resched() check you mentioned to that it can fall into the
> slow path and go to sleep?
Yes, I do think that the outer loop at least should have a test for
need_resche
On Wed, 7 Jan 2009, Gregory Haskins wrote:
> Hi Ingo,
>
> Ingo Molnar wrote:
> > * Gregory Haskins wrote:
> >
> >
> >> Can I ask a simple question in light of all this discussion?
> >>
> >> "Is get_task_struct() really that bad?"
> >>
> >
> > it dirties a cacheline and it also involves
On Wed, 2009-01-07 at 13:58 -0800, Linus Torvalds wrote:
>
> On Wed, 7 Jan 2009, Peter Zijlstra wrote:
> >
> > Do we really have to re-do all that code every loop?
>
> No, you're right, we can just look up the cpu once. Which makes Andrew's
> argument that "probe_kernel_address()" isn't in any
Hi Ingo,
Ingo Molnar wrote:
> * Gregory Haskins wrote:
>
>
>> Can I ask a simple question in light of all this discussion?
>>
>> "Is get_task_struct() really that bad?"
>>
>
> it dirties a cacheline and it also involves atomics.
>
Yes, understood. But we should note we are always goin
On Wed, 2009-01-07 at 23:33 +0100, Ingo Molnar wrote:
> * Gregory Haskins wrote:
>
> > Can I ask a simple question in light of all this discussion?
> >
> > "Is get_task_struct() really that bad?"
>
> it dirties a cacheline and it also involves atomics.
>
> Also, it's a small design cleanliness
* Gregory Haskins wrote:
> Can I ask a simple question in light of all this discussion?
>
> "Is get_task_struct() really that bad?"
it dirties a cacheline and it also involves atomics.
Also, it's a small design cleanliness issue to me: get_task_struct()
impacts the lifetime of an object - an
Andi Kleen wrote:
>> I appreciate this is sample code, but using __get_user() on
>> non-userspace pointers messes up architectures which have separate
>> user/kernel spaces (eg the old 4G/4G split for x86-32). Do we have an
>> appropriate function for kernel space pointers?
>>
>
> probe_kern
On Wed, 2009-01-07 at 13:58 -0800, Linus Torvalds wrote:
>
> On Wed, 7 Jan 2009, Peter Zijlstra wrote:
> >
> > Do we really have to re-do all that code every loop?
>
> No, you're right, we can just look up the cpu once. Which makes Andrew's
> argument that "probe_kernel_address()" isn't in any
On Wed, 7 Jan 2009, Linus Torvalds wrote:
>
> We don't actually care that it only happens once: this all has _known_
> races, and the "cpu_relax()" is a barrier.
I phrased that badly. It's not that it has "known races", it's really that
the whole code sequence is very much written and intende
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
>
> Do we really have to re-do all that code every loop?
No, you're right, we can just look up the cpu once. Which makes Andrew's
argument that "probe_kernel_address()" isn't in any hot path even more
true.
> Also, it would still need to do the funny
* Andrew Morton wrote:
> On Wed, 7 Jan 2009 22:32:22 +0100
> Ingo Molnar wrote:
>
> > > We could do the whole "oldfs = get_fs(); set_fs(KERNEL_DS); ..
> > > set_fs(oldfs);" crud, but it would probably be better to just add an
> > > architected accessor. Especially since it's going to general
On Wed, 2009-01-07 at 12:55 -0800, Linus Torvalds wrote:
> /*
>* Look out! "thread" is an entirely speculative pointer
>* access and not reliable.
>*/
> void loop_while_oncpu(struct mutex *lock, struct thread_struct *thread)
> {
> for (;;) {
On Wed, 7 Jan 2009 22:32:22 +0100
Ingo Molnar wrote:
> > We could do the whole "oldfs = get_fs(); set_fs(KERNEL_DS); ..
> > set_fs(oldfs);" crud, but it would probably be better to just add an
> > architected accessor. Especially since it's going to generally just be a
> >
> > #define get_
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
On Wed, 7 Jan 2009 22:37:40 +0100
Andi Kleen wrote:
> > But we can do that with __get_user(thread_info->cpu) (very unlikely page
> > fault protection due to the possibility of CONFIG_DEBUG_PAGEALLOC) and
> > then validating the cpu. It it's in range, we can use it and verify
> > whether cpu_rq
* Linus Torvalds wrote:
>
>
> On Wed, 7 Jan 2009, Matthew Wilcox wrote:
> >
> > I appreciate this is sample code, but using __get_user() on
> > non-userspace pointers messes up architectures which have separate
> > user/kernel spaces (eg the old 4G/4G split for x86-32). Do we have an
> > app
* Linus Torvalds wrote:
> /*
>* Even if the access succeeded (likely case),
>* the cpu field may no longer be valid. FIXME:
>* this needs to validate that we can do a
>* get_cpu(
> I appreciate this is sample code, but using __get_user() on
> non-userspace pointers messes up architectures which have separate
> user/kernel spaces (eg the old 4G/4G split for x86-32). Do we have an
> appropriate function for kernel space pointers?
probe_kernel_address().
But it's slow.
-A
On Wed, 7 Jan 2009, Matthew Wilcox wrote:
>
> I appreciate this is sample code, but using __get_user() on
> non-userspace pointers messes up architectures which have separate
> user/kernel spaces (eg the old 4G/4G split for x86-32). Do we have an
> appropriate function for kernel space pointers
> But we can do that with __get_user(thread_info->cpu) (very unlikely page
> fault protection due to the possibility of CONFIG_DEBUG_PAGEALLOC) and
> then validating the cpu. It it's in range, we can use it and verify
> whether cpu_rq(cpu)->curr has that thread_info.
>
> So we can do all that l
On Wed, Jan 07, 2009 at 12:55:49PM -0800, Linus Torvalds wrote:
> void loop_while_oncpu(struct mutex *lock, struct thread_struct *thread)
> {
> for (;;) {
> unsigned cpu;
> struct runqueue *rq;
>
> if (lock
On Wed, 7 Jan 2009, Steven Rostedt wrote:
>
> Next comes the issue to know if the owner is still running. Wouldn't we
> need to do something like
>
> if (task_thread_info(cpu_rq(cpu)->curr) == owner)
Yes. After verifying that "cpu" is in a valid range.
> I understand that this should n
On Wed, 7 Jan 2009, Linus Torvalds wrote:
>
> So we can do all that locklessly and optimistically, just going back and
> verifying the results later. This is why "thread_info" is actually a
> better thing to use than "task_struct" - we can look up the cpu in it with
> a simple dereference. We
Ok a few more issues. This never stops.
Here's the basic spinloop:
On Wed, 7 Jan 2009, Peter Zijlstra wrote:
>
> + for (;;) {
> + struct task_struct *l_owner;
> +
> + /* Stop spinning when there's a pending signal. */
> + if (signal_pending_state(task->st
46 matches
Mail list logo
