[RFC/PATCH] sh: Remove cast of atomic_t in ATOMIC_INIT macro

[RFC/PATCH] sh: Remove cast of atomic_t in ATOMIC_INIT macro

[Nobuhiro Iwamatsu]

Hi,

By the update of the trace system (enable CONFIG_TRACING), trace build is not made in SH.
This is macro of trace, and ATOMIC_INIT is used, but becomes the build error with an initialization code.

----
include/trace/events/oom.h:8:13: error: initializer element is not constant
include/trace/events/oom.h:8:13: error: (near initialization for '__tracepoint_oom_score_adj_update')
include/trace/events/oom.h:8:13: error: initializer element is not constant
include/trace/events/oom.h:8:13: error: (near initialization for '__tracepoint_oom_score_adj_update.key')
----

This is caused by that the cast to atomic_t is performed by ATOMIC_INIT of SH.
However, at almost all the architecture, the cast to atomic_t is not performed by ATOMIC_INIT.
When I perform the initialization that is static of the struct, I understand it if the cast to a struct is necessary.
However, it becomes the error when I initialize the struct in the struct.

I show a code following.
-----
typedef struct {
	int counter;
} atomic_t;

struct b
{
	atomic_t data;
};

struct a
{
	struct b x;
};

#define ATOMIC_INIT(i)    ( (atomic_t) { (i) } )
//#define ATOMIC_INIT(i)      { (i) }

static struct a t ={
	((struct b) { .data = ATOMIC_INIT(1)} )
};

static struct b k = { .data = ATOMIC_INIT(1)}
-----

Please teach me how we should correspond this problem.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 arch/sh/include/asm/atomic.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/sh/include/asm/atomic.h b/arch/sh/include/asm/atomic.h
index 37f2f4a..f4c1c20 100644
--- a/arch/sh/include/asm/atomic.h
+++ b/arch/sh/include/asm/atomic.h
@@ -11,7 +11,7 @@
 #include <linux/types.h>
 #include <asm/cmpxchg.h>
 
-#define ATOMIC_INIT(i)	( (atomic_t) { (i) } )
+#define ATOMIC_INIT(i)	{ (i) }
 
 #define atomic_read(v)		(*(volatile int *)&(v)->counter)
 #define atomic_set(v,i)		((v)->counter = (i))
-- 
1.7.9.1

Re: [RFC/PATCH] sh: Remove cast of atomic_t in ATOMIC_INIT macro

[Paul Mundt]

On Mon, Apr 23, 2012 at 11:48:32AM +0900, Nobuhiro Iwamatsu wrote:
> Hi,
> 
> By the update of the trace system (enable CONFIG_TRACING), trace build is not made in SH.
> This is macro of trace, and ATOMIC_INIT is used, but becomes the build error with an initialization code.
> 
> ----
> include/trace/events/oom.h:8:13: error: initializer element is not constant
> include/trace/events/oom.h:8:13: error: (near initialization for '__tracepoint_oom_score_adj_update')
> include/trace/events/oom.h:8:13: error: initializer element is not constant
> include/trace/events/oom.h:8:13: error: (near initialization for '__tracepoint_oom_score_adj_update.key')
> ----
> 
> This is caused by that the cast to atomic_t is performed by ATOMIC_INIT of SH.
> However, at almost all the architecture, the cast to atomic_t is not performed by ATOMIC_INIT.

And on other architectures it is. We don't specifically have any need for
using the cast, so dropping it is probably fine, but there are other
architectures that will either have to do the same or the tracing change
that introduced the build failure has to be rethought.

Re: [RFC/PATCH] sh: Remove cast of atomic_t in ATOMIC_INIT macro

[Nobuhiro Iwamatsu]

2012/4/23 Paul Mundt <lethal@linux-sh.org>:
> On Mon, Apr 23, 2012 at 11:48:32AM +0900, Nobuhiro Iwamatsu wrote:
>> Hi,
>>
>> By the update of the trace system (enable CONFIG_TRACING), trace build is not made in SH.
>> This is macro of trace, and ATOMIC_INIT is used, but becomes the build error with an initialization code.
>>
>> ----
>> include/trace/events/oom.h:8:13: error: initializer element is not constant
>> include/trace/events/oom.h:8:13: error: (near initialization for '__tracepoint_oom_score_adj_update')
>> include/trace/events/oom.h:8:13: error: initializer element is not constant
>> include/trace/events/oom.h:8:13: error: (near initialization for '__tracepoint_oom_score_adj_update.key')
>> ----
>>
>> This is caused by that the cast to atomic_t is performed by ATOMIC_INIT of SH.
>> However, at almost all the architecture, the cast to atomic_t is not performed by ATOMIC_INIT.
>
> And on other architectures it is. We don't specifically have any need for
> using the cast, so dropping it is probably fine, but there are other
> architectures that will either have to do the same or the tracing change
> that introduced the build failure has to be rethought.

Thanks for your comments.

The architecture casting in ATOMIC_INIT has IA64 and Alpha, Parisc
other than SH.
The others do not cast. And these architectures have same problem.

And this problem occurs by STATIC_KEY_INIT_TRUE and
STATIC_KEY_INIT_FALSE of include/linux/jump_label.h.
The architecture by which jumplabel   is defined following,

> #define STATIC_KEY_INIT_TRUE ((struct static_key) \
>     { .enabled = ATOMIC_INIT(1), .entries = (void *)1 })
> #define STATIC_KEY_INIT_FALSE ((struct static_key) \
>     { .enabled = ATOMIC_INIT(0), .entries = (void *)0 })

and does not define following.

> #define STATIC_KEY_INIT_TRUE ((struct static_key) \
>                { .enabled = ATOMIC_INIT(1) })
> #define STATIC_KEY_INIT_FALSE ((struct static_key) \
>                { .enabled = ATOMIC_INIT(0) })

Although a problem is solved by considering it following.

> #define STATIC_KEY_INIT_TRUE {ATOMIC_INIT(1)}
> #define STATIC_KEY_INIT_FALSE {ATOMIC_INIT(0)}

However, since definitions differ by the same macro name,
extendibility is lost.
I think that it is better to remove the cast from ATOMIC_INIT.

Best regards,
  Nobuhiro

-- 
Nobuhiro Iwamatsu

Re: [RFC/PATCH] sh: Remove cast of atomic_t in ATOMIC_INIT macro

[Paul Mundt]

On Wed, Apr 25, 2012 at 12:11:14PM +0900, Nobuhiro Iwamatsu wrote:
> And this problem occurs by STATIC_KEY_INIT_TRUE and
> STATIC_KEY_INIT_FALSE of include/linux/jump_label.h.
> The architecture by which jumplabel   is defined following,
> 
> > #define STATIC_KEY_INIT_TRUE ((struct static_key) \
> >     { .enabled = ATOMIC_INIT(1), .entries = (void *)1 })
> > #define STATIC_KEY_INIT_FALSE ((struct static_key) \
> >     { .enabled = ATOMIC_INIT(0), .entries = (void *)0 })
> 
> and does not define following.
> 
> > #define STATIC_KEY_INIT_TRUE ((struct static_key) \
> >                { .enabled = ATOMIC_INIT(1) })
> > #define STATIC_KEY_INIT_FALSE ((struct static_key) \
> >                { .enabled = ATOMIC_INIT(0) })
> 
> Although a problem is solved by considering it following.
> 
> > #define STATIC_KEY_INIT_TRUE {ATOMIC_INIT(1)}
> > #define STATIC_KEY_INIT_FALSE {ATOMIC_INIT(0)}
> 
> However, since definitions differ by the same macro name,
> extendibility is lost.
> I think that it is better to remove the cast from ATOMIC_INIT.
> 
Yes, agreed. I'll queue up the patch, thanks.

Re: [RFC/PATCH] sh: Remove cast of atomic_t in ATOMIC_INIT macro

[Simon Horman]

On Fri, Apr 27, 2012 at 09:35:50AM +0900, Paul Mundt wrote:
> On Wed, Apr 25, 2012 at 12:11:14PM +0900, Nobuhiro Iwamatsu wrote:
> > And this problem occurs by STATIC_KEY_INIT_TRUE and
> > STATIC_KEY_INIT_FALSE of include/linux/jump_label.h.
> > The architecture by which jumplabel   is defined following,
> > 
> > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \
> > >     { .enabled = ATOMIC_INIT(1), .entries = (void *)1 })
> > > #define STATIC_KEY_INIT_FALSE ((struct static_key) \
> > >     { .enabled = ATOMIC_INIT(0), .entries = (void *)0 })
> > 
> > and does not define following.
> > 
> > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \
> > >                { .enabled = ATOMIC_INIT(1) })
> > > #define STATIC_KEY_INIT_FALSE ((struct static_key) \
> > >                { .enabled = ATOMIC_INIT(0) })
> > 
> > Although a problem is solved by considering it following.
> > 
> > > #define STATIC_KEY_INIT_TRUE {ATOMIC_INIT(1)}
> > > #define STATIC_KEY_INIT_FALSE {ATOMIC_INIT(0)}
> > 
> > However, since definitions differ by the same macro name,
> > extendibility is lost.
> > I think that it is better to remove the cast from ATOMIC_INIT.
> > 
> Yes, agreed. I'll queue up the patch, thanks.

Excellent.