mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] compiler.h: simplify data_race() macro
@ 2024-06-24 11:49 Alexey Dobriyan
  2024-06-24 12:27 ` Marco Elver
  0 siblings, 1 reply; 9+ messages in thread
From: Alexey Dobriyan @ 2024-06-24 11:49 UTC (permalink / raw)
  To: Thomas Gleixner, Marco Elver; +Cc: linux-kernel, akpm

Use auto type deduction and comma expression to decrease macro expansion
size.

__unqual_scalar_typeof() is quite wordy macro by itself.

"expr" can be arbitrary complex so not expanding it twice is good.
Should be faster too because type is deduced only once
from the initializer.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 include/linux/compiler.h |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -200,10 +200,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
  */
 #define data_race(expr)							\
 ({									\
-	__unqual_scalar_typeof(({ expr; })) __v = ({			\
-		__kcsan_disable_current();				\
-		expr;							\
-	});								\
+	__auto_type __v = (__kcsan_disable_current(), expr);		\
 	__kcsan_enable_current();					\
 	__v;								\
 })

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] compiler.h: simplify data_race() macro
  2024-06-24 11:49 [PATCH] compiler.h: simplify data_race() macro Alexey Dobriyan
@ 2024-06-24 12:27 ` Marco Elver
  2024-06-24 14:37   ` Alexey Dobriyan
  2024-06-24 14:42   ` Alexey Dobriyan
  0 siblings, 2 replies; 9+ messages in thread
From: Marco Elver @ 2024-06-24 12:27 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Thomas Gleixner, linux-kernel, akpm

On Mon, 24 Jun 2024 at 13:49, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> Use auto type deduction and comma expression to decrease macro expansion
> size.
>
> __unqual_scalar_typeof() is quite wordy macro by itself.
>
> "expr" can be arbitrary complex so not expanding it twice is good.
> Should be faster too because type is deduced only once
> from the initializer.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

Thanks for cleaning up.  That code certainly predates the availability
of __auto_type. Although if I recall correctly, __unqual_scalar_typeof
became the first user of _Generic (the first C11 keyword we used in
the kernel?), but we used some ifdef to still support ancient
compilers initially (that definitely also didn't have __auto_type).

Reviewed-by: Marco Elver <elver@google.com>

Which tree is this for?

> ---
>
>  include/linux/compiler.h |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -200,10 +200,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>   */
>  #define data_race(expr)                                                        \
>  ({                                                                     \
> -       __unqual_scalar_typeof(({ expr; })) __v = ({                    \
> -               __kcsan_disable_current();                              \
> -               expr;                                                   \
> -       });                                                             \
> +       __auto_type __v = (__kcsan_disable_current(), expr);            \
>         __kcsan_enable_current();                                       \
>         __v;                                                            \
>  })

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] compiler.h: simplify data_race() macro
  2024-06-24 12:27 ` Marco Elver
@ 2024-06-24 14:37   ` Alexey Dobriyan
  2024-06-24 14:41     ` Marco Elver
  2024-06-24 14:42   ` Alexey Dobriyan
  1 sibling, 1 reply; 9+ messages in thread
From: Alexey Dobriyan @ 2024-06-24 14:37 UTC (permalink / raw)
  To: Marco Elver; +Cc: Thomas Gleixner, linux-kernel, akpm

On Mon, Jun 24, 2024 at 02:27:43PM +0200, Marco Elver wrote:
> On Mon, 24 Jun 2024 at 13:49, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >
> > Use auto type deduction and comma expression to decrease macro expansion
> > size.
> >
> > __unqual_scalar_typeof() is quite wordy macro by itself.
> >
> > "expr" can be arbitrary complex so not expanding it twice is good.
> > Should be faster too because type is deduced only once
> > from the initializer.
> >
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> 
> Thanks for cleaning up.  That code certainly predates the availability
> of __auto_type. Although if I recall correctly, __unqual_scalar_typeof
> became the first user of _Generic (the first C11 keyword we used in
> the kernel?), but we used some ifdef to still support ancient
> compilers initially (that definitely also didn't have __auto_type).
> 
> Reviewed-by: Marco Elver <elver@google.com>
> 
> Which tree is this for?

This is against mainline. Should not really matter, -next has the same code.

> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -200,10 +200,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> >   */
> >  #define data_race(expr)                                                        \
> >  ({                                                                     \
> > -       __unqual_scalar_typeof(({ expr; })) __v = ({                    \
> > -               __kcsan_disable_current();                              \
> > -               expr;                                                   \
> > -       });                                                             \
> > +       __auto_type __v = (__kcsan_disable_current(), expr);            \
> >         __kcsan_enable_current();                                       \
> >         __v;                                                            \
> >  })

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] compiler.h: simplify data_race() macro
  2024-06-24 14:37   ` Alexey Dobriyan
@ 2024-06-24 14:41     ` Marco Elver
  0 siblings, 0 replies; 9+ messages in thread
From: Marco Elver @ 2024-06-24 14:41 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Thomas Gleixner, linux-kernel, akpm

On Mon, 24 Jun 2024 at 16:38, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> On Mon, Jun 24, 2024 at 02:27:43PM +0200, Marco Elver wrote:
> > On Mon, 24 Jun 2024 at 13:49, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > >
> > > Use auto type deduction and comma expression to decrease macro expansion
> > > size.
> > >
> > > __unqual_scalar_typeof() is quite wordy macro by itself.
> > >
> > > "expr" can be arbitrary complex so not expanding it twice is good.
> > > Should be faster too because type is deduced only once
> > > from the initializer.
> > >
> > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> >
> > Thanks for cleaning up.  That code certainly predates the availability
> > of __auto_type. Although if I recall correctly, __unqual_scalar_typeof
> > became the first user of _Generic (the first C11 keyword we used in
> > the kernel?), but we used some ifdef to still support ancient
> > compilers initially (that definitely also didn't have __auto_type).
> >
> > Reviewed-by: Marco Elver <elver@google.com>
> >
> > Which tree is this for?
>
> This is against mainline. Should not really matter, -next has the same code.

I meant which maintainer/tree should pick this up?

> > > --- a/include/linux/compiler.h
> > > +++ b/include/linux/compiler.h
> > > @@ -200,10 +200,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> > >   */
> > >  #define data_race(expr)                                                        \
> > >  ({                                                                     \
> > > -       __unqual_scalar_typeof(({ expr; })) __v = ({                    \
> > > -               __kcsan_disable_current();                              \
> > > -               expr;                                                   \
> > > -       });                                                             \
> > > +       __auto_type __v = (__kcsan_disable_current(), expr);            \
> > >         __kcsan_enable_current();                                       \
> > >         __v;                                                            \
> > >  })

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] compiler.h: simplify data_race() macro
  2024-06-24 12:27 ` Marco Elver
  2024-06-24 14:37   ` Alexey Dobriyan
@ 2024-06-24 14:42   ` Alexey Dobriyan
  2024-06-24 15:09     ` Marco Elver
  1 sibling, 1 reply; 9+ messages in thread
From: Alexey Dobriyan @ 2024-06-24 14:42 UTC (permalink / raw)
  To: Marco Elver; +Cc: Thomas Gleixner, linux-kernel, akpm

On Mon, Jun 24, 2024 at 02:27:43PM +0200, Marco Elver wrote:
> On Mon, 24 Jun 2024 at 13:49, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >
> > Use auto type deduction and comma expression to decrease macro expansion
> > size.
> >
> > __unqual_scalar_typeof() is quite wordy macro by itself.
> >
> > "expr" can be arbitrary complex so not expanding it twice is good.
> > Should be faster too because type is deduced only once
> > from the initializer.
> >
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> 
> Thanks for cleaning up.  That code certainly predates the availability
> of __auto_type. Although if I recall correctly, __unqual_scalar_typeof
> became the first user of _Generic (the first C11 keyword we used in
> the kernel?), but we used some ifdef to still support ancient
> compilers initially (that definitely also didn't have __auto_type).
> 
> Reviewed-by: Marco Elver <elver@google.com>
> 
> Which tree is this for?
> 
> > ---
> >
> >  include/linux/compiler.h |    5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -200,10 +200,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> >   */
> >  #define data_race(expr)                                                        \
> >  ({                                                                     \
> > -       __unqual_scalar_typeof(({ expr; })) __v = ({                    \
> > -               __kcsan_disable_current();                              \
> > -               expr;                                                   \
> > -       });                                                             \
> > +       __auto_type __v = (__kcsan_disable_current(), expr);            \
> >         __kcsan_enable_current();                                       \
> >         __v;                                                            \
> >  })

I just realized, comma expression should not be necesary.
-Wdeclaration-after-statement prohibited simple

	({
		__kcsan_disable_current
		auto v = (expr);
		__kcsan_enable_current
		v;
	})

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] compiler.h: simplify data_race() macro
  2024-06-24 14:42   ` Alexey Dobriyan
@ 2024-06-24 15:09     ` Marco Elver
  2024-06-24 15:39       ` [PATCH v2] " Alexey Dobriyan
  0 siblings, 1 reply; 9+ messages in thread
From: Marco Elver @ 2024-06-24 15:09 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Thomas Gleixner, linux-kernel, akpm

On Mon, 24 Jun 2024 at 16:43, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> On Mon, Jun 24, 2024 at 02:27:43PM +0200, Marco Elver wrote:
> > On Mon, 24 Jun 2024 at 13:49, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > >
> > > Use auto type deduction and comma expression to decrease macro expansion
> > > size.
> > >
> > > __unqual_scalar_typeof() is quite wordy macro by itself.
> > >
> > > "expr" can be arbitrary complex so not expanding it twice is good.
> > > Should be faster too because type is deduced only once
> > > from the initializer.
> > >
> > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> >
> > Thanks for cleaning up.  That code certainly predates the availability
> > of __auto_type. Although if I recall correctly, __unqual_scalar_typeof
> > became the first user of _Generic (the first C11 keyword we used in
> > the kernel?), but we used some ifdef to still support ancient
> > compilers initially (that definitely also didn't have __auto_type).
> >
> > Reviewed-by: Marco Elver <elver@google.com>
> >
> > Which tree is this for?
> >
> > > ---
> > >
> > >  include/linux/compiler.h |    5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > --- a/include/linux/compiler.h
> > > +++ b/include/linux/compiler.h
> > > @@ -200,10 +200,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> > >   */
> > >  #define data_race(expr)                                                        \
> > >  ({                                                                     \
> > > -       __unqual_scalar_typeof(({ expr; })) __v = ({                    \
> > > -               __kcsan_disable_current();                              \
> > > -               expr;                                                   \
> > > -       });                                                             \
> > > +       __auto_type __v = (__kcsan_disable_current(), expr);            \
> > >         __kcsan_enable_current();                                       \
> > >         __v;                                                            \
> > >  })
>
> I just realized, comma expression should not be necesary.
> -Wdeclaration-after-statement prohibited simple
>
>         ({
>                 __kcsan_disable_current
>                 auto v = (expr);
>                 __kcsan_enable_current
>                 v;
>         })

Even better.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] compiler.h: simplify data_race() macro
  2024-06-24 15:09     ` Marco Elver
@ 2024-06-24 15:39       ` Alexey Dobriyan
  2024-06-24 15:41         ` Marco Elver
  2024-06-28 15:03         ` David Laight
  0 siblings, 2 replies; 9+ messages in thread
From: Alexey Dobriyan @ 2024-06-24 15:39 UTC (permalink / raw)
  To: Marco Elver; +Cc: Thomas Gleixner, linux-kernel, akpm

-Wdeclaration-after-statement used since forever required statement
expressions to inject __kcsan_disable_current(), __kcsan_enable_current()
to mark data race. Now that it is gone, make macro expansion simpler.

__unqual_scalar_typeof() is wordy macro by itself.
"expr" is expanded twice.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 include/linux/compiler.h |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -200,10 +200,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
  */
 #define data_race(expr)							\
 ({									\
-	__unqual_scalar_typeof(({ expr; })) __v = ({			\
-		__kcsan_disable_current();				\
-		expr;							\
-	});								\
+	__kcsan_disable_current();					\
+	__auto_type __v = (expr);					\
 	__kcsan_enable_current();					\
 	__v;								\
 })

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] compiler.h: simplify data_race() macro
  2024-06-24 15:39       ` [PATCH v2] " Alexey Dobriyan
@ 2024-06-24 15:41         ` Marco Elver
  2024-06-28 15:03         ` David Laight
  1 sibling, 0 replies; 9+ messages in thread
From: Marco Elver @ 2024-06-24 15:41 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Thomas Gleixner, linux-kernel, akpm

On Mon, 24 Jun 2024 at 17:39, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> -Wdeclaration-after-statement used since forever required statement
> expressions to inject __kcsan_disable_current(), __kcsan_enable_current()
> to mark data race. Now that it is gone, make macro expansion simpler.
>
> __unqual_scalar_typeof() is wordy macro by itself.
> "expr" is expanded twice.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

Reviewed-by: Marco Elver <elver@google.com>

> ---
>
>  include/linux/compiler.h |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -200,10 +200,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>   */
>  #define data_race(expr)                                                        \
>  ({                                                                     \
> -       __unqual_scalar_typeof(({ expr; })) __v = ({                    \
> -               __kcsan_disable_current();                              \
> -               expr;                                                   \
> -       });                                                             \
> +       __kcsan_disable_current();                                      \
> +       __auto_type __v = (expr);                                       \
>         __kcsan_enable_current();                                       \
>         __v;                                                            \
>  })

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2] compiler.h: simplify data_race() macro
  2024-06-24 15:39       ` [PATCH v2] " Alexey Dobriyan
  2024-06-24 15:41         ` Marco Elver
@ 2024-06-28 15:03         ` David Laight
  1 sibling, 0 replies; 9+ messages in thread
From: David Laight @ 2024-06-28 15:03 UTC (permalink / raw)
  To: 'Alexey Dobriyan', Marco Elver
  Cc: Thomas Gleixner, linux-kernel, akpm

From: Alexey Dobriyan
> Sent: 24 June 2024 16:40
> 
> -Wdeclaration-after-statement used since forever required statement
> expressions to inject __kcsan_disable_current(), __kcsan_enable_current()
> to mark data race. Now that it is gone, make macro expansion simpler.
> 
> __unqual_scalar_typeof() is wordy macro by itself.
> "expr" is expanded twice.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  include/linux/compiler.h |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -200,10 +200,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>   */
>  #define data_race(expr)							\
>  ({									\
> -	__unqual_scalar_typeof(({ expr; })) __v = ({			\
> -		__kcsan_disable_current();				\
> -		expr;							\
> -	});								\
> +	__kcsan_disable_current();					\
> +	__auto_type __v = (expr);					\
>  	__kcsan_enable_current();					\
>  	__v;								\
>  })

I think you can remove a lot of the tabs...

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-06-28 15:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-24 11:49 [PATCH] compiler.h: simplify data_race() macro Alexey Dobriyan
2024-06-24 12:27 ` Marco Elver
2024-06-24 14:37   ` Alexey Dobriyan
2024-06-24 14:41     ` Marco Elver
2024-06-24 14:42   ` Alexey Dobriyan
2024-06-24 15:09     ` Marco Elver
2024-06-24 15:39       ` [PATCH v2] " Alexey Dobriyan
2024-06-24 15:41         ` Marco Elver
2024-06-28 15:03         ` David Laight

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®