more efficient current_is_keventd macro? [was Re: [lhcs-devel] Re: Kthread_create() never returns when called from worker_thread]

more efficient current_is_keventd macro? [was Re: [lhcs-devel] Re: Kthread_create() never returns when called from worker_thread]

[Srivatsa Vaddagiri]

Hi Ingo,
	current_is_keventd() macro checks "current" with the per-cpu
thread of _all_ possible cpus attached to keventd_wq. Can't it just check 
against the per-cpu thread of _current_ cpu alone (since the per-cpu workqueue
threads are anyway bound only to their cpus)?  

int current_is_keventd(void)
{
        struct cpu_workqueue_struct *cwq;
-       int cpu;
+       int cpu = smp_processor_id();

        BUG_ON(!keventd_wq);

-       for_each_cpu(cpu) {
-               cwq = keventd_wq->cpu_wq + cpu;
-               if (current == cwq->thread)
-                       return 1;
-       }

-       return 0;

+	cwq = keventd_wq->cpu_wq + cpu;
+	if (current == cwq->thread)
+		return 1;
+	else
+		return 0;
}


----- Forwarded message from Rusty Russell <rusty@rustcorp.com.au> -----

Subject: Re: [lhcs-devel] Re: Kthread_create() never returns when called
	from worker_thread
From: Rusty Russell <rusty@rustcorp.com.au>
To: vatsa@in.ibm.com
Cc: rusty@au1.ibm.com,
	"Keshavamurthy, Anil S" <anil.s.keshavamurthy@intel.com>,
	lhcs-devel@lists.sourceforge.net, "Raj, Ashok" <ashok.raj@intel.com>,
	"Shah, Rajesh" <rajesh.shah@intel.com>
X-Mailer: Ximian Evolution 1.4.5 
Date: Mon, 08 Mar 2004 11:29:27 +1100

On Wed, 2004-03-03 at 20:08, Srivatsa Vaddagiri wrote: 
> On Tue, Mar 02, 2004 at 12:10:08PM +1100, Rusty Russell wrote:
> >  	/* If we're being called to start the first workqueue, we
> >  	 * can't use keventd. */
> > -	if (!keventd_up())
> > +	if (!keventd_up() || current_is_keventd())
> >  		work.func(work.data);
> >  	else {
> >  		schedule_work(&work);
> 
> I noticed that current_is_keventd() check actually goes and compares
> "current" with all the per-cpu threads attached to keventd() workqueue ..
> Is that really necessary? Can't it compare with the per-cpu thread
> attached to _current_ cpu only? 

You'd think so, but it's not my code.  I believe you are correct though.

Cheers,
Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
_______________________________________________
lhcs-devel mailing list
lhcs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lhcs-devel

----- End forwarded message -----

-- 


Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560017

Re: more efficient current_is_keventd macro? [was Re: [lhcs-devel] Re: Kthread_create() never returns when called from worker_thread]

[Andrew Morton]

Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote:
>
> 	current_is_keventd() macro checks "current" with the per-cpu
> thread of _all_ possible cpus attached to keventd_wq. Can't it just check 
> against the per-cpu thread of _current_ cpu alone (since the per-cpu workqueue
> threads are anyway bound only to their cpus)?  

Seems that way, yes.

> int current_is_keventd(void)
> {
>         struct cpu_workqueue_struct *cwq;
> -       int cpu;
> +       int cpu = smp_processor_id();
> 
>         BUG_ON(!keventd_wq);
> 
> -       for_each_cpu(cpu) {
> -               cwq = keventd_wq->cpu_wq + cpu;
> -               if (current == cwq->thread)
> -                       return 1;
> -       }
> 
> -       return 0;
> 
> +	cwq = keventd_wq->cpu_wq + cpu;
> +	if (current == cwq->thread)
> +		return 1;
> +	else
> +		return 0;
> }

Is racy in the presence of preemption.  Please replace smp_processor_id()
with get_cpu(), stick a put_cpu() at the end, avoid having two function
return points, test it and send me the diff?

Re: more efficient current_is_keventd macro? [was Re: [lhcs-devel] Re: Kthread_create() never returns when called from worker_thread]

[Rusty Russell]

On Tue, 2004-03-09 at 09:36, Andrew Morton wrote:
> Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote:
> > int current_is_keventd(void)
> > {
> > +       int cpu = smp_processor_id();
> > +	cwq = keventd_wq->cpu_wq + cpu;
> > +	if (current == cwq->thread)
> > +		return 1;
> > +	else
> > +		return 0;
> > }
> 
> Is racy in the presence of preemption.

Actually, it's not, because if current *is* keventd we're nailed to the
CPU, and if it's not, we're going to return false either way.

But it *should* be fixed, simply as an example to others.

Cheers,
Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

Re: more efficient current_is_keventd macro? [was Re: [lhcs-devel] Re: Kthread_create() never returns when called from worker_thread]

[Srivatsa Vaddagiri]

On Mon, Mar 08, 2004 at 02:36:58PM -0800, Andrew Morton wrote:
> Is racy in the presence of preemption.  Please replace smp_processor_id()
> with get_cpu(), stick a put_cpu() at the end, avoid having two function
> return points, test it and send me the diff?

Hi Andrew,
	I had considered preemption and had thought using just
smp_processor_id() should be safe, since anyway the per-cpu kevent
thread is bound to its cpu alone.

Patch below is booted/tested against 2.6.4-rc2-mm1 on a 4-way x86 SMP box.
For testing, I basically called call_usermodehelper from inside a 
work function (activated using schedule_work) and checked that 
current_is_keventd() macro was returning true.

--- workqueue.c.org	2004-03-09 11:34:30.000000000 +0530
+++ workqueue.c	2004-03-09 19:06:51.000000000 +0530
@@ -374,16 +374,17 @@
 int current_is_keventd(void)
 {
 	struct cpu_workqueue_struct *cwq;
-	int cpu;
+	int cpu = smp_processor_id();
+	int ret = 0;
 
 	BUG_ON(!keventd_wq);
 
-	for_each_cpu(cpu) {
-		cwq = keventd_wq->cpu_wq + cpu;
-		if (current == cwq->thread)
-			return 1;
-	}
-	return 0;
+	cwq = keventd_wq->cpu_wq + cpu;
+	if (current == cwq->thread)
+		ret = 1;
+	
+	return ret;
+
 }
 
 #ifdef CONFIG_HOTPLUG_CPU

-- 


Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560017