lkml.org 
[lkml]   [2010]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [RFC local_t removal V1 1/4] Add add_local() and add_local_return()
Date
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.

> Also, separating all these in sub-files will make it a bit difficult to
> pinpoint errors that would arise from using a bad combination of, e.g.
> generic add with a non-protected read or set.

Yes.

> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6/include/asm-generic/add-local-generic.h 2010-01-05 15:36:02.000000000 -0600
> > @@ -0,0 +1,40 @@
> > +#ifndef __ASM_GENERIC_ADD_LOCAL_GENERIC_H
> > +#define __ASM_GENERIC_ADD_LOCAL_GENERIC_H
Why split the file between asm-generic/add-local.h and add-local-generic.h?
I don't see how any architecture could use one but not the other.

> > +#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.

Arnd


\
 
 \ /
  Last update: 2010-01-07 14:49    [W:0.138 / U:0.600 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site