Don't know who put this in for 2.5.38.
I realize that using xchg() makes you 'leet. But doing an atomic op
where none is required is suboptimal and confusing.
Hey, at least it wasn't:
if (likely(tsk = xchg(task_cache + cpu, tsk))) {
Cheers,
Rusty.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.38/kernel/fork.c working-2.5.38-unxchg/kernel/fork.c
--- linux-2.5.38/kernel/fork.c 2002-09-21 13:55:19.000000000 +1000
+++ working-2.5.38-unxchg/kernel/fork.c 2002-09-23 11:00:31.000000000 +1000
@@ -64,11 +64,12 @@ void __put_task_struct(struct task_struc
} else {
int cpu = smp_processor_id();
- tsk = xchg(task_cache + cpu, tsk);
+ tsk = task_cache[cpu];
if (tsk) {
free_thread_info(tsk->thread_info);
kmem_cache_free(task_struct_cachep,tsk);
}
+ task_cache[cpu] = current;
}
}
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
On Mon, 2002-09-23 at 14:31, Ingo Molnar wrote:
> the attached patch (against BK-curr) fixes all xchg()'s and a preemption
> bug.
Thanks, Ingo.
> --- linux/kernel/fork.c.orig Mon Sep 23 20:28:36 2002
> +++ linux/kernel/fork.c Mon Sep 23 20:30:02 2002
> @@ -64,11 +64,12 @@
> } else {
> int cpu = smp_processor_id();
>
> - tsk = xchg(task_cache + cpu, tsk);
> + tsk = task_cache[cpu];
> if (tsk) {
> free_thread_info(tsk->thread_info);
> kmem_cache_free(task_struct_cachep,tsk);
> }
> + task_cache[cpu] = current;
> }
> }
I think you need get/put_cpu() here, too?
Robert Love
On Mon, 23 Sep 2002, Rusty Russell wrote:
> - tsk = xchg(task_cache + cpu, tsk);
> + tsk = task_cache[cpu];
this was me - i forgot that this are per-CPU fields. (hey and i wrote the
original task_cache code so there's no excuse :-)
the attached patch (against BK-curr) fixes all xchg()'s and a preemption
bug.
Ingo
--- linux/kernel/fork.c.orig Mon Sep 23 20:28:36 2002
+++ linux/kernel/fork.c Mon Sep 23 20:30:02 2002
@@ -64,11 +64,12 @@
} else {
int cpu = smp_processor_id();
- tsk = xchg(task_cache + cpu, tsk);
+ tsk = task_cache[cpu];
if (tsk) {
free_thread_info(tsk->thread_info);
kmem_cache_free(task_struct_cachep,tsk);
}
+ task_cache[cpu] = current;
}
}
@@ -126,8 +127,11 @@
{
struct task_struct *tsk;
struct thread_info *ti;
+ int cpu = get_cpu();
- tsk = xchg(task_cache + smp_processor_id(), NULL);
+ tsk = task_cache[cpu];
+ task_cache[cpu] = NULL;
+ put_cpu();
if (!tsk) {
ti = alloc_thread_info();
if (!ti)
On 23 Sep 2002, Robert Love wrote:
> > + task_cache[cpu] = current;
> > }
> > }
>
> I think you need get/put_cpu() here, too?
you are right - new patch attached.
Ingo
--- linux/kernel/fork.c.orig Mon Sep 23 20:28:36 2002
+++ linux/kernel/fork.c Mon Sep 23 20:55:08 2002
@@ -62,13 +62,15 @@
free_thread_info(tsk->thread_info);
kmem_cache_free(task_struct_cachep,tsk);
} else {
- int cpu = smp_processor_id();
+ int cpu = get_cpu();
- tsk = xchg(task_cache + cpu, tsk);
+ tsk = task_cache[cpu];
if (tsk) {
free_thread_info(tsk->thread_info);
kmem_cache_free(task_struct_cachep,tsk);
}
+ task_cache[cpu] = current;
+ put_cpu();
}
}
@@ -126,8 +128,11 @@
{
struct task_struct *tsk;
struct thread_info *ti;
+ int cpu = get_cpu();
- tsk = xchg(task_cache + smp_processor_id(), NULL);
+ tsk = task_cache[cpu];
+ task_cache[cpu] = NULL;
+ put_cpu();
if (!tsk) {
ti = alloc_thread_info();
if (!ti)
Hi!
> Don't know who put this in for 2.5.38.
>
> I realize that using xchg() makes you 'leet. But doing an atomic op
> where none is required is suboptimal and confusing.
Well, atomic op is also more expensive. i think ingo did this. Ingo, is
patch below safe? It is faster *and* it works on 386.
Pavel
> diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.38/kernel/fork.c working-2.5.38-unxchg/kernel/fork.c
> --- linux-2.5.38/kernel/fork.c 2002-09-21 13:55:19.000000000 +1000
> +++ working-2.5.38-unxchg/kernel/fork.c 2002-09-23 11:00:31.000000000 +1000
> @@ -64,11 +64,12 @@ void __put_task_struct(struct task_struc
> } else {
> int cpu = smp_processor_id();
>
> - tsk = xchg(task_cache + cpu, tsk);
> + tsk = task_cache[cpu];
> if (tsk) {
> free_thread_info(tsk->thread_info);
> kmem_cache_free(task_struct_cachep,tsk);
> }
> + task_cache[cpu] = current;
> }
> }
--
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.