All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@us.ibm.com>
To: Paul Jackson <pj@sgi.com>
Cc: akpm@osdl.org, Eric Dumazet <dada1@cosmosbay.com>,
	linux-kernel@vger.kernel.org,
	Nick Piggin <nickpiggin@yahoo.com.au>,
	Simon Derr <Simon.Derr@bull.net>, Andi Kleen <ak@suse.de>,
	Christoph Lameter <clameter@sgi.com>
Subject: Re: [PATCH 04/04] Cpuset: skip rcu check if task is in root cpuset
Date: Fri, 16 Dec 2005 09:52:01 -0800	[thread overview]
Message-ID: <20051216175201.GA24876@us.ibm.com> (raw)
In-Reply-To: <20051214084049.21054.34108.sendpatchset@jackhammer.engr.sgi.com>

On Wed, Dec 14, 2005 at 12:40:49AM -0800, Paul Jackson wrote:
> For systems that aren't using cpusets, but have them
> CONFIG_CPUSET enabled in their kernel (eventually this
> may be most distribution kernels), this patch removes
> even the minimal rcu_read_lock() from the memory page
> allocation path.
> 
> Actually, it removes that rcu call for any task that is
> in the root cpuset (top_cpuset), which on systems not
> actively using cpusets, is all tasks.
> 
> We don't need the rcu check for tasks in the top_cpuset,
> because the top_cpuset is statically allocated, so at
> no risk of being freed out from underneath us.
> 
> Signed-off-by: Paul Jackson <pj@sgi.com>
> 
> ---
> 
>  kernel/cpuset.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> --- 2.6.15-rc3-mm1.orig/kernel/cpuset.c	2005-12-13 18:14:42.529952708 -0800
> +++ 2.6.15-rc3-mm1/kernel/cpuset.c	2005-12-13 20:54:26.323911532 -0800
> @@ -647,10 +647,15 @@ void cpuset_update_task_memory_state()
>  	struct task_struct *tsk = current;
>  	struct cpuset *cs;
>  
> -	rcu_read_lock();
> -	cs = rcu_dereference(tsk->cpuset);
> -	my_cpusets_mem_gen = cs->mems_generation;
> -	rcu_read_unlock();
> +	if (tsk->cpuset == &top_cpuset) {
> +		/* Don't need rcu for top_cpuset.  It's never freed. */
> +		my_cpusets_mem_gen = top_cpuset.mems_generation;
> +	} else {
> +		rcu_read_lock();
> +		cs = rcu_dereference(tsk->cpuset);
> +		my_cpusets_mem_gen = cs->mems_generation;
> +		rcu_read_unlock();
> +	}

Hmmm...  In non-CONFIG_PREEMPT kernels on non-Alpha CPUs, rcu_read_lock(),
rcu_read_unlock(), and rcu_reference() do nothing.  So in such cases, the
above code will be slower than unconditionally using RCU read side.

In CONFIG_PREEMPT kernels on non-Alpha CPUs, rcu_read_lock() and
rcu_read_unlock() are private non-atomic increment and decrement,
so are likely to be about the same cost as the branch.

In CONFIG_PREEMPT_RT kernels, this optimization would currently buy
you something, but might not once we get rcu_read_lock() and
rcu_read_unlock() more fully optimized.

So I am not convinced that this optimization is worthwhile.

							Thanx, Paul

>  
>  	if (my_cpusets_mem_gen != tsk->cpuset_mems_generation) {
>  		down(&callback_sem);
> 
> -- 
>                           I won't rest till it's the best ...
>                           Programmer, Linux Scalability
>                           Paul Jackson <pj@sgi.com> 1.650.933.1373
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

  reply	other threads:[~2005-12-16 17:51 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-12-14  8:40 [PATCH 01/04] Cpuset: remove rcu slab cache optimization Paul Jackson
2005-12-14  8:40 ` [PATCH 02/04] Cpuset: use rcu directly optimization Paul Jackson
2005-12-16 17:55   ` Paul E. McKenney
2005-12-16 20:22     ` Paul Jackson
2005-12-14  8:40 ` [PATCH 03/04] Cpuset: mark number_of_cpusets read_mostly Paul Jackson
2005-12-14  8:40 ` [PATCH 04/04] Cpuset: skip rcu check if task is in root cpuset Paul Jackson
2005-12-16 17:52   ` Paul E. McKenney [this message]
2005-12-16 20:06     ` Paul Jackson
2005-12-17 16:47       ` Paul E. McKenney
2005-12-19 14:48         ` Paul Jackson
2005-12-19 16:04           ` CONFIG_DEBUG_PREEMPT (was: [PATCH 04/04] Cpuset: skip rcu check ...) Paul Jackson
2005-12-19 16:39             ` Greg Edwards

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=20051216175201.GA24876@us.ibm.com \
    --to=paulmck@us.ibm.com \
    --cc=Simon.Derr@bull.net \
    --cc=ak@suse.de \
    --cc=akpm@osdl.org \
    --cc=clameter@sgi.com \
    --cc=dada1@cosmosbay.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nickpiggin@yahoo.com.au \
    --cc=pj@sgi.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.