public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Lameter <cl@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC local_t removal V1 1/4] Add add_local() and add_local_return()
Date: Thu, 7 Jan 2010 08:57:57 -0500	[thread overview]
Message-ID: <20100107135757.GA6011@Krystal> (raw)
In-Reply-To: <201001071445.50416.arnd@arndb.de>

* Arnd Bergmann (arnd@arndb.de) wrote:
> On Tuesday 05 January 2010, Mathieu Desnoyers wrote:
> > 
> > The problem I see here is that with ~5-6 operations, we will end up
> > having 20*5 = 100 headers only for this. Can we combine these in a
> > single header file instead ? local.h wasn't bad in this respect.
> 
> I have an old patch that I was planning to dig out for 2.6.34,
> which autogenerates arch/*/include/foo.h files that only contain
> "#include <asm-generic/foo.h>".
> 
> I guess this would be sufficient to avoid the overload with all
> these header files.

Well, given we already have local.h, I am not completely sure that this
whole exercise is giving us.

[...]

> > > +#include <linux/types.h>
> > > +
> > > +extern unsigned long wrong_size_add_local(volatile void *ptr);
> > > +
> > > +/*
> > > + * Generic version of __add_return_local (disables interrupts). Takes an
> > > + * unsigned long parameter, supporting various types of architectures.
> > > + */
> > > +static inline unsigned long __add_return_local_generic(volatile void *ptr,
> > > +		unsigned long value, int size)
> 
> You could probably lose the 'volatile' here, if you want to discourage
> marking data as volatile in the code.
> 
> > > +{
> > > +	unsigned long flags, r;
> > > +
> > > +	/*
> > > +	 * Sanity checking, compile-time.
> > > +	 */
> > > +	if (size == 8 && sizeof(unsigned long) != 8)
> > > +		wrong_size_add_local(ptr);
> 
> It can be BUILD_BUG_ON if you move it to the outer macro.
> 
> > > +	local_irq_save(flags);
> > > +	switch (size) {
> > > +	case 1: r = (*((u8 *)ptr) += value);
> > > +		break;
> > > +	case 2: r = (*((u16 *)ptr) += value);
> > > +		break;
> > > +	case 4: r = (*((u32 *)ptr) += value);
> > > +		break;
> > > +	case 8: r = (*((u64 *)ptr) += value);
> > > +		break;
> 
> But I think here you actually need to add the volatile in order
> to make these atomic assignments.

Yes, you are right. If we ever try to access these variables from a
remote CPU with a load (but not with any concurrent store operation, as
this would be semantically invalid), then the volatile is important.

Mathieu

> 
> 	Arnd

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

  reply	other threads:[~2010-01-07 13:58 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-05 22:04 [RFC local_t removal V1 0/4] Remove local_t Christoph Lameter
2010-01-05 22:04 ` [RFC local_t removal V1 1/4] Add add_local() and add_local_return() Christoph Lameter
2010-01-05 22:17   ` Mike Frysinger
2010-01-05 22:29     ` Christoph Lameter
2010-01-06 19:23       ` Mike Frysinger
2010-01-07 17:03         ` Christoph Lameter
2010-01-05 22:49   ` Mathieu Desnoyers
2010-01-07 13:45     ` Arnd Bergmann
2010-01-07 13:57       ` Mathieu Desnoyers [this message]
2010-01-07 14:22         ` Arnd Bergmann
2010-01-07 17:07     ` Christoph Lameter
2010-01-05 22:04 ` [RFC local_t removal V1 2/4] Replace local_t use in trace subsystem Christoph Lameter
2010-01-05 22:57   ` Mathieu Desnoyers
2010-01-07 17:15     ` Christoph Lameter
2010-01-14  2:56   ` Steven Rostedt
2010-01-14 14:49     ` Christoph Lameter
2010-01-05 22:04 ` [RFC local_t removal V1 3/4] Optimized add_local() Christoph Lameter
2010-01-05 22:59   ` Mathieu Desnoyers
2010-01-07 17:16     ` Christoph Lameter
2010-01-05 22:04 ` [RFC local_t removal V1 4/4] Remove local_t support Christoph Lameter
2010-01-05 22:23 ` [RFC local_t removal V1 0/4] Remove local_t Mathieu Desnoyers
2010-01-05 22:34   ` Christoph Lameter
2010-01-05 22:45     ` Mathieu Desnoyers
2010-01-07 17:05       ` Christoph Lameter

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=20100107135757.GA6011@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --cc=arnd@arndb.de \
    --cc=cl@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox