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
next prev parent 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 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.