LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Venki Pallipadi <venkatesh.pallipadi@intel.com>
Cc: Andi Kleen <ak@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>, "Brown, Len" <len.brown@intel.com>,
	Adam Belay <abelay@novell.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [PATCH] x86: Simplify cpu_idle_wait
Date: Sat, 9 Feb 2008 10:33:21 +0100 (CET)	[thread overview]
Message-ID: <alpine.LFD.1.00.0802091029400.3145@apollo.tec.linutronix.de> (raw)
In-Reply-To: <20080208172430.GA8302@linux-os.sc.intel.com>

On Fri, 8 Feb 2008, Venki Pallipadi wrote:

> On Fri, Feb 08, 2008 at 11:28:48AM +0100, Andi Kleen wrote:
> > 
> > > -	set_cpus_allowed(current, tmp);
> > > +	smp_mb();
> > > +	/* kick all the CPUs so that they exit out of pm_idle */
> > > +	smp_call_function(do_nothing, NULL, 0, 0);
> > 
> > I think the last argument (wait) needs to be 1 to make sure it is 
> > synchronous (for 32/64) Otherwise the patch looks great.
> 
> Yes. Below is the updated patch

Applied. Thanks,

	 tglx

> Earlier commit 40d6a146629b98d8e322b6f9332b182c7cbff3df
> added smp_call_function in cpu_idle_wait() to kick cpus that are in tickless
> idle. Looking at cpu_idle_wait code at that time, code seemed to be
> over-engineered for a case which is rarely used (while changing idle handler).
> 
> Below is a simplified version of cpu_idle_wait, which just makes
> a dummy smp_call_function to all cpus, to make them come out of old idle handler
> and start using the new idle handler. It eliminates code in the idle loop
> to handle cpu_idle_wait.
> 
> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> 
> Index: linux-2.6.25-rc/arch/x86/kernel/process_32.c
> ===================================================================
> --- linux-2.6.25-rc.orig/arch/x86/kernel/process_32.c
> +++ linux-2.6.25-rc/arch/x86/kernel/process_32.c
> @@ -82,7 +82,6 @@ unsigned long thread_saved_pc(struct tas
>   */
>  void (*pm_idle)(void);
>  EXPORT_SYMBOL(pm_idle);
> -static DEFINE_PER_CPU(unsigned int, cpu_idle_state);
>  
>  void disable_hlt(void)
>  {
> @@ -181,9 +180,6 @@ void cpu_idle(void)
>  		while (!need_resched()) {
>  			void (*idle)(void);
>  
> -			if (__get_cpu_var(cpu_idle_state))
> -				__get_cpu_var(cpu_idle_state) = 0;
> -
>  			check_pgt_cache();
>  			rmb();
>  			idle = pm_idle;
> @@ -208,40 +204,19 @@ static void do_nothing(void *unused)
>  {
>  }
>  
> +/*
> + * cpu_idle_wait - Used to ensure that all the CPUs discard old value of
> + * pm_idle and update to new pm_idle value. Required while changing pm_idle
> + * handler on SMP systems.
> + *
> + * Caller must have changed pm_idle to the new value before the call. Old
> + * pm_idle value will not be used by any CPU after the return of this function.
> + */
>  void cpu_idle_wait(void)
>  {
> -	unsigned int cpu, this_cpu = get_cpu();
> -	cpumask_t map, tmp = current->cpus_allowed;
> -
> -	set_cpus_allowed(current, cpumask_of_cpu(this_cpu));
> -	put_cpu();
> -
> -	cpus_clear(map);
> -	for_each_online_cpu(cpu) {
> -		per_cpu(cpu_idle_state, cpu) = 1;
> -		cpu_set(cpu, map);
> -	}
> -
> -	__get_cpu_var(cpu_idle_state) = 0;
> -
> -	wmb();
> -	do {
> -		ssleep(1);
> -		for_each_online_cpu(cpu) {
> -			if (cpu_isset(cpu, map) && !per_cpu(cpu_idle_state, cpu))
> -				cpu_clear(cpu, map);
> -		}
> -		cpus_and(map, map, cpu_online_map);
> -		/*
> -		 * We waited 1 sec, if a CPU still did not call idle
> -		 * it may be because it is in idle and not waking up
> -		 * because it has nothing to do.
> -		 * Give all the remaining CPUS a kick.
> -		 */
> -		smp_call_function_mask(map, do_nothing, 0, 0);
> -	} while (!cpus_empty(map));
> -
> -	set_cpus_allowed(current, tmp);
> +	smp_mb();
> +	/* kick all the CPUs so that they exit out of pm_idle */
> +	smp_call_function(do_nothing, NULL, 0, 1);
>  }
>  EXPORT_SYMBOL_GPL(cpu_idle_wait);
>  
> Index: linux-2.6.25-rc/arch/x86/kernel/process_64.c
> ===================================================================
> --- linux-2.6.25-rc.orig/arch/x86/kernel/process_64.c
> +++ linux-2.6.25-rc/arch/x86/kernel/process_64.c
> @@ -64,7 +64,6 @@ EXPORT_SYMBOL(boot_option_idle_override)
>   */
>  void (*pm_idle)(void);
>  EXPORT_SYMBOL(pm_idle);
> -static DEFINE_PER_CPU(unsigned int, cpu_idle_state);
>  
>  static ATOMIC_NOTIFIER_HEAD(idle_notifier);
>  
> @@ -139,41 +138,19 @@ static void do_nothing(void *unused)
>  {
>  }
>  
> +/*
> + * cpu_idle_wait - Used to ensure that all the CPUs discard old value of
> + * pm_idle and update to new pm_idle value. Required while changing pm_idle
> + * handler on SMP systems.
> + *
> + * Caller must have changed pm_idle to the new value before the call. Old
> + * pm_idle value will not be used by any CPU after the return of this function.
> + */
>  void cpu_idle_wait(void)
>  {
> -	unsigned int cpu, this_cpu = get_cpu();
> -	cpumask_t map, tmp = current->cpus_allowed;
> -
> -	set_cpus_allowed(current, cpumask_of_cpu(this_cpu));
> -	put_cpu();
> -
> -	cpus_clear(map);
> -	for_each_online_cpu(cpu) {
> -		per_cpu(cpu_idle_state, cpu) = 1;
> -		cpu_set(cpu, map);
> -	}
> -
> -	__get_cpu_var(cpu_idle_state) = 0;
> -
> -	wmb();
> -	do {
> -		ssleep(1);
> -		for_each_online_cpu(cpu) {
> -			if (cpu_isset(cpu, map) &&
> -					!per_cpu(cpu_idle_state, cpu))
> -				cpu_clear(cpu, map);
> -		}
> -		cpus_and(map, map, cpu_online_map);
> -		/*
> -		 * We waited 1 sec, if a CPU still did not call idle
> -		 * it may be because it is in idle and not waking up
> -		 * because it has nothing to do.
> -		 * Give all the remaining CPUS a kick.
> -		 */
> -		smp_call_function_mask(map, do_nothing, 0, 0);
> -	} while (!cpus_empty(map));
> -
> -	set_cpus_allowed(current, tmp);
> +	smp_mb();
> +	/* kick all the CPUs so that they exit out of pm_idle */
> +	smp_call_function(do_nothing, NULL, 0, 1);
>  }
>  EXPORT_SYMBOL_GPL(cpu_idle_wait);
>  
> @@ -215,9 +192,6 @@ void cpu_idle (void)
>  		while (!need_resched()) {
>  			void (*idle)(void);
>  
> -			if (__get_cpu_var(cpu_idle_state))
> -				__get_cpu_var(cpu_idle_state) = 0;
> -
>  			tick_nohz_stop_sched_tick();
>  
>  			rmb();
> 

  parent reply	other threads:[~2008-02-09  9:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-08  2:27 [RFC PATCH] kick sleeping idle CPUS on cpu_idle_wait Steven Rostedt
2008-01-08  3:33 ` Andi Kleen
2008-01-09 20:42   ` [PATCH] Kick CPUS that might be sleeping in cpus_idle_wait Steven Rostedt
2008-01-09 22:12     ` Steven Rostedt
2008-01-10 13:45       ` Ingo Molnar
2008-01-10 14:43         ` Steven Rostedt
2008-01-10 17:31           ` Pallipadi, Venkatesh
2008-01-10 18:03             ` Steven Rostedt
2008-01-09 23:42     ` Andrew Morton
2008-01-10  0:05       ` Steven Rostedt
2008-01-10  0:12     ` Pallipadi, Venkatesh
     [not found]     ` <B5B0CFF685D7DF46A05CF1678CFB42ED20E00653@orsmsx423.amr.corp.intel.com>
2008-02-08  1:05       ` [PATCH] x86: Simplify cpu_idle_wait Venki Pallipadi
2008-02-08 10:28         ` Andi Kleen
2008-02-08 17:24           ` Venki Pallipadi
2008-02-08 18:45             ` Andi Kleen
2008-02-09  9:33             ` Thomas Gleixner [this message]
2008-04-10 16:49               ` Venki Pallipadi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.1.00.0802091029400.3145@apollo.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=a.p.zijlstra@chello.nl \
    --cc=abelay@novell.com \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --cc=venkatesh.pallipadi@intel.com \
    --subject='Re: [PATCH] x86: Simplify cpu_idle_wait' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).