RE: [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset.
diff mbox

Message ID 2236FBA76BA1254E88B949DDB74E612B41BE01A7@IRSMSX102.ger.corp.intel.com
State New
Headers show

Commit Message

Reshetova, Elena Oct. 19, 2016, 8:21 a.m. UTC
Signed-off-by: Colin Vidal <colin@cvidal.org>
---
 include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------
 include/linux/atomic.h            | 55 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 25 deletions(-)

 /* atomic_inc_return_relaxed */
@@ -113,6 +118,11 @@
 #define  atomic_inc_return(...)						\
 	__atomic_op_fence(atomic_inc_return, __VA_ARGS__)  #endif
+
+#ifndef atomic_inc_return_wrap
+#define  atomic_inc_return_wrap(...)					\
+	__atomic_op_fence(atomic_inc_return_wrap, __VA_ARGS__) #endif
 #endif /* atomic_inc_return_relaxed */
 
 /* atomic_sub_return_relaxed */
@@ -137,6 +147,11 @@
 #define  atomic_sub_return(...)						\
 	__atomic_op_fence(atomic_sub_return, __VA_ARGS__)  #endif
+
+#ifndef atomic_sub_return_wrap_relaxed

ER: Same as above

+#define atomic_sub_return_wrap(...)		                        \
+	__atomic_op_fence(atomic_sub_return_wrap, __VA_ARGS__) #endif
 #endif /* atomic_sub_return_relaxed */
 
 /* atomic_dec_return_relaxed */
@@ -161,6 +176,11 @@
 #define  atomic_dec_return(...)						\
 	__atomic_op_fence(atomic_dec_return, __VA_ARGS__)  #endif
+
+#ifndef atomic_dec_return_wrap
+#define  atomic_dec_return_wrap(...)					\
+	__atomic_op_fence(atomic_dec_return, __VA_ARGS__) #endif

ER: Another typo? atomic_dec_return_wrap?


Actually we will hopefully send next rfc today-tomorrow and we plan to include the above and other general declarations, so hopefully you don't need to carry this part with your rfc anymore!

Best Regards,
Elena.

Comments

Greg KH Oct. 19, 2016, 8:31 a.m. UTC | #1
On Wed, Oct 19, 2016 at 08:21:06AM +0000, Reshetova, Elena wrote:
> Signed-off-by: Colin Vidal <colin@cvidal.org>

No changelog at all?

> ---
>  include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------
>  include/linux/atomic.h            | 55 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 insertions(+), 25 deletions(-)
> 
> diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
> index 790cb00..94d712b 100644
> --- a/include/asm-generic/atomic-long.h
> +++ b/include/asm-generic/atomic-long.h
> @@ -46,6 +46,30 @@ typedef atomic_t atomic_long_wrap_t;
>  
>  #endif
>  
> +#ifndef CONFIG_HARDENED_ATOMIC
> +#define atomic_read_wrap(v) atomic_read(v) #define atomic_set_wrap(v, 
> +i) atomic_set((v), (i)) #define atomic_add_wrap(i, v) atomic_add((i), 
> +(v)) #define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), 
> +(i), (j)) #define atomic_sub_wrap(i, v) atomic_sub((i), (v)) #define 
> +atomic_inc_wrap(v) atomic_inc(v) #define atomic_inc_and_test_wrap(v) 
> +atomic_inc_and_test(v) #ifndef atomic_inc_return_wrap #define 
> +atomic_inc_return_wrap(v) atomic_inc_return(v) #endif #ifndef 
> +atomic_add_return_wrap #define atomic_add_return_wrap(i, v) 
> +atomic_add_return((i), (v)) #endif #define atomic_dec_wrap(v) 
> +atomic_dec(v) #ifndef atomic_xchg_wrap #define atomic_xchg_wrap(v, i) 
> +atomic_xchg((v), (i)) #endif #define atomic_long_inc_wrap(v) 
> +atomic_long_inc(v) #define atomic_long_dec_wrap(v) atomic_long_dec(v) 
> +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) #define 
> +atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) #endif 
> +/* CONFIG_HARDENED_ATOMIC */

That doesn't look correct to me at all, does it to you?

thanks,

greg k-h
Colin Vidal Oct. 19, 2016, 8:58 a.m. UTC | #2
Hi Greg,

On Wed, 2016-10-19 at 10:31 +0200, Greg KH wrote:
> On Wed, Oct 19, 2016 at 08:21:06AM +0000, Reshetova, Elena wrote:
> > 
> > Signed-off-by: Colin Vidal <colin@cvidal.org>
> 
> No changelog at all?

My mistake, I fix that for the next review (some informations was
actually in the cover-letter).

> > 
> > ---
> >  include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------
> >  include/linux/atomic.h            | 55 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 85 insertions(+), 25 deletions(-)
> > 
> > diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
> > index 790cb00..94d712b 100644
> > --- a/include/asm-generic/atomic-long.h
> > +++ b/include/asm-generic/atomic-long.h
> > @@ -46,6 +46,30 @@ typedef atomic_t atomic_long_wrap_t;
> >  
> >  #endif
> >  
> > +#ifndef CONFIG_HARDENED_ATOMIC
> > +#define atomic_read_wrap(v) atomic_read(v)
> > +#define atomic_set_wrap(v, i) atomic_set((v), (i))
> > +#define atomic_add_wrap(i, v) atomic_add((i), (v))
> > +#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j))
> > +#define atomic_sub_wrap(i, v) atomic_sub((i), (v))
> > +#define atomic_inc_wrap(v) atomic_inc(v)
> > +#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v)
> > +#ifndef atomic_inc_return_wrap
> > +#define atomic_inc_return_wrap(v) atomic_inc_return(v)
> > +#endif
> > +#ifndef atomic_add_return_wrap
> > +#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v))
> > +#endif
> > +#define atomic_dec_wrap(v) atomic_dec(v)
> > +#ifndef atomic_xchg_wrap
> > +#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i))
> > +#endif
> > +#define atomic_long_inc_wrap(v) atomic_long_inc(v)
> > +#define atomic_long_dec_wrap(v) atomic_long_dec(v)
> > +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n)
> > +#define atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n)
> > +#endif /* CONFIG_HARDENED_ATOMIC */
> 
> That doesn't look correct to me at all, does it to you?

That patch will be removed in next version since it will be not needed
anymore (Elena's RFC will include those changes). However, I wonder
what is obviously wrong here? (in order to avoid it in futures
patches). Complex nested #ifdef/#ifndef?

Thanks,

Colin
Greg KH Oct. 19, 2016, 9:16 a.m. UTC | #3
On Wed, Oct 19, 2016 at 10:58:09AM +0200, Colin Vidal wrote:
> > > +#ifndef CONFIG_HARDENED_ATOMIC
> > > +#define atomic_read_wrap(v) atomic_read(v)
> > > +#define atomic_set_wrap(v, i) atomic_set((v), (i))
> > > +#define atomic_add_wrap(i, v) atomic_add((i), (v))
> > > +#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j))
> > > +#define atomic_sub_wrap(i, v) atomic_sub((i), (v))
> > > +#define atomic_inc_wrap(v) atomic_inc(v)
> > > +#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v)
> > > +#ifndef atomic_inc_return_wrap
> > > +#define atomic_inc_return_wrap(v) atomic_inc_return(v)
> > > +#endif
> > > +#ifndef atomic_add_return_wrap
> > > +#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v))
> > > +#endif
> > > +#define atomic_dec_wrap(v) atomic_dec(v)
> > > +#ifndef atomic_xchg_wrap
> > > +#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i))
> > > +#endif
> > > +#define atomic_long_inc_wrap(v) atomic_long_inc(v)
> > > +#define atomic_long_dec_wrap(v) atomic_long_dec(v)
> > > +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n)
> > > +#define atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n)
> > > +#endif /* CONFIG_HARDENED_ATOMIC */
> > 
> > That doesn't look correct to me at all, does it to you?
> 
> That patch will be removed in next version since it will be not needed
> anymore (Elena's RFC will include those changes). However, I wonder
> what is obviously wrong here? (in order to avoid it in futures
> patches). Complex nested #ifdef/#ifndef?

Odd, the version I had had all of the line-ends gone, and it was wrapped
horribly, but this email shows it correctly.  email is fun...

greg k-h

Patch
diff mbox

diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
index 790cb00..94d712b 100644
--- a/include/asm-generic/atomic-long.h
+++ b/include/asm-generic/atomic-long.h
@@ -46,6 +46,30 @@  typedef atomic_t atomic_long_wrap_t;
 
 #endif
 
+#ifndef CONFIG_HARDENED_ATOMIC
+#define atomic_read_wrap(v) atomic_read(v) #define atomic_set_wrap(v, 
+i) atomic_set((v), (i)) #define atomic_add_wrap(i, v) atomic_add((i), 
+(v)) #define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), 
+(i), (j)) #define atomic_sub_wrap(i, v) atomic_sub((i), (v)) #define 
+atomic_inc_wrap(v) atomic_inc(v) #define atomic_inc_and_test_wrap(v) 
+atomic_inc_and_test(v) #ifndef atomic_inc_return_wrap #define 
+atomic_inc_return_wrap(v) atomic_inc_return(v) #endif #ifndef 
+atomic_add_return_wrap #define atomic_add_return_wrap(i, v) 
+atomic_add_return((i), (v)) #endif #define atomic_dec_wrap(v) 
+atomic_dec(v) #ifndef atomic_xchg_wrap #define atomic_xchg_wrap(v, i) 
+atomic_xchg((v), (i)) #endif #define atomic_long_inc_wrap(v) 
+atomic_long_inc(v) #define atomic_long_dec_wrap(v) atomic_long_dec(v) 
+#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) #define 
+atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) #endif 
+/* CONFIG_HARDENED_ATOMIC */
+
 #define ATOMIC_LONG_READ_OP(mo, suffix)						\
 static inline long atomic_long_read##mo##suffix(const atomic_long##suffix##_t *l)\
 {									\
@@ -104,6 +128,12 @@  ATOMIC_LONG_ADD_SUB_OP(sub, _release,)  #define atomic_long_cmpxchg(l, old, new) \
 	(ATOMIC_LONG_PFX(_cmpxchg)((ATOMIC_LONG_PFX(_t) *)(l), (old), (new)))
 
+#ifdef CONFIG_HARDENED_ATOMIC
+#define atomic_long_cmpxchg_wrap(l, old, new)				\
+	(ATOMIC_LONG_PFX(_cmpxchg_wrap)((ATOMIC_LONG_PFX(_wrap_t) *)(l),\
+					(old), (new)))
+#endif
+
 #define atomic_long_xchg_relaxed(v, new) \
 	(ATOMIC_LONG_PFX(_xchg_relaxed)((ATOMIC_LONG_PFX(_t) *)(v), (new)))  #define atomic_long_xchg_acquire(v, new) \ @@ -291,29 +321,4 @@ static inline long atomic_long_add_unless(atomic_long_t *l, long a, long u)  #define atomic_long_inc_not_zero(l) \
 	ATOMIC_LONG_PFX(_inc_not_zero)((ATOMIC_LONG_PFX(_t) *)(l))
 
-#ifndef CONFIG_HARDENED_ATOMIC
-#define atomic_read_wrap(v) atomic_read(v) -#define atomic_set_wrap(v, i) atomic_set((v), (i)) -#define atomic_add_wrap(i, v) atomic_add((i), (v)) -#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) -#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) -#define atomic_inc_wrap(v) atomic_inc(v) -#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) -#define atomic_inc_return_wrap(v) atomic_inc_return(v) -#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) -#define atomic_dec_wrap(v) atomic_dec(v) -#define atomic_cmpxchg_wrap(v, o, n) atomic_cmpxchg((v), (o), (n)) -#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) -#define atomic_long_read_wrap(v) atomic_long_read(v) -#define atomic_long_set_wrap(v, i) atomic_long_set((v), (i)) -#define atomic_long_add_wrap(i, v) atomic_long_add((i), (v)) -#define atomic_long_sub_wrap(i, v) atomic_long_sub((i), (v)) -#define atomic_long_inc_wrap(v) atomic_long_inc(v) -#define atomic_long_add_return_wrap(i, v) atomic_long_add_return((i), (v)) -#define atomic_long_inc_return_wrap(v) atomic_long_inc_return(v) -#define atomic_long_sub_and_test_wrap(i, v) atomic_long_sub_and_test((i), (v)) -#define atomic_long_dec_wrap(v) atomic_long_dec(v) -#define atomic_long_xchg_wrap(v, i) atomic_long_xchg((v), (i)) -#endif /* CONFIG_HARDENED_ATOMIC */
-
 #endif  /*  _ASM_GENERIC_ATOMIC_LONG_H  */ diff --git a/include/linux/atomic.h b/include/linux/atomic.h index b5817c8..be16ea1 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -89,6 +89,11 @@ 
 #define  atomic_add_return(...)						\
 	__atomic_op_fence(atomic_add_return, __VA_ARGS__)  #endif
+
+#ifndef atomic_add_return_wrap_relaxed

ER: I guess this is a typo, should be ifndef atomic_add_return_wrap

+#define atomic_add_return_wrap(...)		                        \
+	__atomic_op_fence(atomic_add_return_wrap, __VA_ARGS__) #endif
 #endif /* atomic_add_return_relaxed */