Re: [RFC local_t removal V1 1/4] Add add_local() andadd_local_return()

From: Mathieu Desnoyers
Date: Thu Jan 07 2010 - 08:58:13 EST


* Arnd Bergmann (arnd@xxxxxxxx) 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/