All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Miller <davem@davemloft.net>,
	rostedt@goodmis.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, paulus@samba.org,
	benh@kernel.crashing.org, linux-ia64@vger.kernel.org,
	linux-s390@vger.kernel.org,
	Christoph Lameter <christoph@lameter.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Martin Bligh <mbligh@google.com>
Subject: Re: local_add_return
Date: Wed, 24 Dec 2008 13:53:03 -0500	[thread overview]
Message-ID: <20081224185302.GA16467@Krystal> (raw)
In-Reply-To: <200812242212.57007.rusty@rustcorp.com.au>

* Rusty Russell (rusty@rustcorp.com.au) wrote:
> On Tuesday 23 December 2008 05:13:28 Mathieu Desnoyers wrote:
> > > I can be convinced, but I'll need more than speculation.  Assuming
> > > local_long_atomic_t, can you produce a patch which uses it somewhere else?
> > 
> > I had this patch applying over Christoph Lameter's vm tree last
> > February. It did accelerate the slub fastpath allocator by using
> > cmpxchg_local rather than disabling interrupts. cmpxchg_local is not
> > using the local_t type, but behaves similarly to local_cmpxchg.
> > 
> > http://lkml.org/lkml/2008/2/28/568
> 
> OK, I'll buy that.  So we split local_t into a counter and an atomic type.
> 
> > I know that
> > local_counter_long_t and local_atomic_long_t are painful to write, but
> > that would follow the current atomic_t vs atomic_long_t semantics. Hm ?
> 
> OK, I've looked at how they're used, to try to figure out whether long
> is the right thing.  Counters generally want to be long, but I was in doubt
> about atomics; yet grep shows that atomic_long_t is quite popular.  Then
> I hit struct nfs_iostats which would want a u64 and a long.  I don't think
> we want local_counter_u64 etc.
> 
> Just thinking out loud, perhaps a new *type* is the wrong direction?  How
> about a set of macros which take a fundamental type, such as:
> 
> 	DECLARE_LOCAL_COUNTER(type, name);
> 	local_counter_inc(type, addr);
> 	...
> 	DECLARE_LOCAL_ATOMIC(type, name);
> 	local_atomic_add_return(type, addr);
> 
> This allows pointers, u32, u64, long, etc.  If a 32-bit arch can't do 64-bit
> local_counter_inc easily, at least the hairy 64-bit code can be eliminated at
> compile time.
> 
> Or maybe that's overdesign?
> Rusty.

Yeah, I also thought of this, but I am not sure every architecture
provides primitives to modify u16 or u8 data atomically like x86 does.
But yes, I remember hearing Christoph Lameter being interested to use
unsigned char or short atomic counters for the vm allocator in the past.
The rationale was mostly that he wanted to keep a counter in a very
small data type, expecting to "poll" the counter periodically (e.g.
every X counter increment) and sum the total somewhere else.

So I think it would be the right design in the end if we want to allow
wider use of such atomic primitives for counters w/o interrupts
disabled. And I would propose we use a BUILD_BUG_ON() when the
architecture does not support an atomic operation on a specific type.
We should also document which type sizes are supported portably and
which are architecture-specific.

Or, as you say, maybe it's overdesign ? If we have to pick something
simple, just supporting "long" would be a good start.

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Miller <davem@davemloft.net>,
	rostedt@goodmis.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, paulus@samba.org,
	benh@kernel.crashing.org, linux-ia64@vger.kernel.org,
	linux-s390@vger.kernel.org,
	Christoph Lameter <christoph@lameter.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Martin Bligh <mbligh@google.com>
Subject: Re: local_add_return
Date: Wed, 24 Dec 2008 18:53:03 +0000	[thread overview]
Message-ID: <20081224185302.GA16467@Krystal> (raw)
In-Reply-To: <200812242212.57007.rusty@rustcorp.com.au>

* Rusty Russell (rusty@rustcorp.com.au) wrote:
> On Tuesday 23 December 2008 05:13:28 Mathieu Desnoyers wrote:
> > > I can be convinced, but I'll need more than speculation.  Assuming
> > > local_long_atomic_t, can you produce a patch which uses it somewhere else?
> > 
> > I had this patch applying over Christoph Lameter's vm tree last
> > February. It did accelerate the slub fastpath allocator by using
> > cmpxchg_local rather than disabling interrupts. cmpxchg_local is not
> > using the local_t type, but behaves similarly to local_cmpxchg.
> > 
> > http://lkml.org/lkml/2008/2/28/568
> 
> OK, I'll buy that.  So we split local_t into a counter and an atomic type.
> 
> > I know that
> > local_counter_long_t and local_atomic_long_t are painful to write, but
> > that would follow the current atomic_t vs atomic_long_t semantics. Hm ?
> 
> OK, I've looked at how they're used, to try to figure out whether long
> is the right thing.  Counters generally want to be long, but I was in doubt
> about atomics; yet grep shows that atomic_long_t is quite popular.  Then
> I hit struct nfs_iostats which would want a u64 and a long.  I don't think
> we want local_counter_u64 etc.
> 
> Just thinking out loud, perhaps a new *type* is the wrong direction?  How
> about a set of macros which take a fundamental type, such as:
> 
> 	DECLARE_LOCAL_COUNTER(type, name);
> 	local_counter_inc(type, addr);
> 	...
> 	DECLARE_LOCAL_ATOMIC(type, name);
> 	local_atomic_add_return(type, addr);
> 
> This allows pointers, u32, u64, long, etc.  If a 32-bit arch can't do 64-bit
> local_counter_inc easily, at least the hairy 64-bit code can be eliminated at
> compile time.
> 
> Or maybe that's overdesign?
> Rusty.

Yeah, I also thought of this, but I am not sure every architecture
provides primitives to modify u16 or u8 data atomically like x86 does.
But yes, I remember hearing Christoph Lameter being interested to use
unsigned char or short atomic counters for the vm allocator in the past.
The rationale was mostly that he wanted to keep a counter in a very
small data type, expecting to "poll" the counter periodically (e.g.
every X counter increment) and sum the total somewhere else.

So I think it would be the right design in the end if we want to allow
wider use of such atomic primitives for counters w/o interrupts
disabled. And I would propose we use a BUILD_BUG_ON() when the
architecture does not support an atomic operation on a specific type.
We should also document which type sizes are supported portably and
which are architecture-specific.

Or, as you say, maybe it's overdesign ? If we have to pick something
simple, just supporting "long" would be a good start.

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  reply	other threads:[~2008-12-24 18:53 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-15 13:47 local_add_return Steven Rostedt
2008-12-16  6:33 ` local_add_return Rusty Russell
2008-12-16  6:57   ` local_add_return David Miller
2008-12-16  7:13   ` local_add_return David Miller
2008-12-16 22:38     ` local_add_return Rusty Russell
2008-12-16 22:50       ` local_add_return Rusty Russell
2008-12-16 23:25       ` local_add_return Luck, Tony
2008-12-16 23:25         ` local_add_return Luck, Tony
2008-12-16 23:43       ` local_add_return Heiko Carstens
2008-12-16 23:43         ` local_add_return Heiko Carstens
2008-12-16 23:59       ` local_add_return Eric Dumazet
2008-12-16 23:59         ` local_add_return Eric Dumazet
2008-12-17  0:01       ` local_add_return Mathieu Desnoyers
2008-12-17  0:01         ` local_add_return Mathieu Desnoyers
2008-12-18 22:52         ` local_add_return Rusty Russell
2008-12-18 22:53           ` local_add_return Rusty Russell
2008-12-19  3:35           ` local_add_return Mathieu Desnoyers
2008-12-19  3:35             ` local_add_return Mathieu Desnoyers
2008-12-19  5:54             ` local_add_return Rusty Russell
2008-12-19  5:54               ` local_add_return Rusty Russell
2008-12-19 17:06               ` local_add_return Mathieu Desnoyers
2008-12-19 17:06                 ` local_add_return Mathieu Desnoyers
2008-12-20  1:33                 ` local_add_return Rusty Russell
2008-12-20  1:45                   ` local_add_return Rusty Russell
2008-12-20  1:33                   ` local_add_return Rusty Russell
2008-12-22 18:43                   ` local_add_return Mathieu Desnoyers
2008-12-22 18:43                     ` local_add_return Mathieu Desnoyers
2008-12-24 11:42                     ` local_add_return Rusty Russell
2008-12-24 11:54                       ` local_add_return Rusty Russell
2008-12-24 18:53                       ` Mathieu Desnoyers [this message]
2008-12-24 18:53                         ` local_add_return Mathieu Desnoyers
2008-12-16 16:25   ` local_add_return Mathieu Desnoyers
2008-12-17 11:23     ` local_add_return Rusty Russell

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=20081224185302.GA16467@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=christoph@lameter.com \
    --cc=davem@davemloft.net \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mbligh@google.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=rostedt@goodmis.org \
    --cc=rusty@rustcorp.com.au \
    /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.