lkml.org 
[lkml]   [2011]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] atomic: cleanup asm-generic atomic*.h inclusion
On Fri, Jun 17, 2011 at 14:42, Arun Sharma wrote:
> On 6/17/11 11:06 AM, Mike Frysinger wrote:
>> fixes one thing while breaking another.  linux/atomic.h includes
>> asm/atomic.h which includes asm-generic/atomic.h which includes
>> asm-generic/atomic-long.h which needs atomic_add_unless(), but that
>> isnt provided until after the asm/atomic.h include in linux/atomic.h.
>>
>> but linux/atomic.h needs asm/atomic.h before atomic_add_unless()
>> because it relies on the new __atomic_add_unless().
>
> Right. I think we need to get rid of that include as well:

ok, then we get to your proposed asm-generic/atomic.h change not being
correct. instead of deleting atomic_add_unless(), it probably should
redo it to __atomic_add_unless(). people using asm-generic/atomic.h
are not defining __atomic_add_unless() themselves.

CC arch/blackfin/kernel/asm-offsets.s
In file included from include/linux/spinlock.h:387,
from include/linux/seqlock.h:29,
from include/linux/time.h:8,
from include/linux/timex.h:56,
from include/linux/sched.h:57,
from arch/blackfin/kernel/asm-offsets.c:10:
include/linux/atomic.h: In function ‘atomic_add_unless’:
include/linux/atomic.h:17: error: implicit declaration of function
‘__atomic_add_unless’
make[1]: *** [arch/blackfin/kernel/asm-offsets.s] Error 1

>> having linux/atomic.h and asm-generic/atomic.h just strikes me as
>> wrong.  the point of asm-generic is to unify things, but now we have
>> two places to unify things without 0 indication as to which is for
>> which ?  i'm wondering if we shouldnt convert all arches to
>> asm-generic/atomic.h and then add your new logic there and just skip
>> this whole linux/atomic.h mess.
>
> I believe the logic is:
>
> <asm-generic/atomic.h> shared code for simple archs that don't want to
> define their own primitives. Only used by 4 archs.

i think the 4 is only really because asm-generic/atomic.h is so new.
those arches are also new and started with it, or did work to convert
over. i imagine a lot of the existing arches could convert over to
it.

> <linux/atomic.h> a header file that's usable in machine independent kernel
> code.

if that's the direction we want to take things, then this needs to be
documented at the top of both files. i also see quite a lot of
primitives in asm-generic/atomic.h that would be better placed using
your definitions in linux/atomic.h.

i would refine your asm-generic/atomic.h definition as "simple C
definitions useful for UP systems only"

> My guess is that if <linux/atomic.h> existed in 2006, people would've added
> shared code over there.

i'd propose that the guy adding linux/atomic.h was lazy and punked out
rather than doing real work

> Also, the code in <asm-generic/atomic.h> is not truly generic:

sure it is. it'll work on any arch in a UP setup. it (currently)
does not support SMP by design.
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2011-06-17 21:09    [W:0.055 / U:0.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site