Remove asm/xchg.h which is included more than once
Signed-off-by: Brajeswar Ghosh <[email protected]>
---
arch/alpha/include/asm/cmpxchg.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
index 6c7c39452471..bcbdac0744f9 100644
--- a/arch/alpha/include/asm/cmpxchg.h
+++ b/arch/alpha/include/asm/cmpxchg.h
@@ -36,7 +36,6 @@
#undef ____cmpxchg
#define ____xchg(type, args...) __xchg ##type(args)
#define ____cmpxchg(type, args...) __cmpxchg ##type(args)
-#include <asm/xchg.h>
/*
* The leading and the trailing memory barriers guarantee that these
--
2.17.1
On Fri, Nov 02, 2018 at 08:56:37PM +0530, Brajeswar Ghosh wrote:
> Remove asm/xchg.h which is included more than once
>
> Signed-off-by: Brajeswar Ghosh <[email protected]>
> ---
> arch/alpha/include/asm/cmpxchg.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
> index 6c7c39452471..bcbdac0744f9 100644
> --- a/arch/alpha/include/asm/cmpxchg.h
> +++ b/arch/alpha/include/asm/cmpxchg.h
> @@ -36,7 +36,6 @@
> #undef ____cmpxchg
> #define ____xchg(type, args...) __xchg ##type(args)
> #define ____cmpxchg(type, args...) __cmpxchg ##type(args)
> -#include <asm/xchg.h>
It's amazing the number of times we get a patch to remove that.
Instead of just automatically removing a second include of a
header file, why don't you take a closer look to see what it
actually does?
Cheers,
Michael.
Hi Michael,
On Fri, Nov 2, 2018 at 11:59 PM Michael Cree <[email protected]> wrote:
>
> On Fri, Nov 02, 2018 at 08:56:37PM +0530, Brajeswar Ghosh wrote:
> > Remove asm/xchg.h which is included more than once
> >
> > Signed-off-by: Brajeswar Ghosh <[email protected]>
> > ---
> > arch/alpha/include/asm/cmpxchg.h | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
> > index 6c7c39452471..bcbdac0744f9 100644
> > --- a/arch/alpha/include/asm/cmpxchg.h
> > +++ b/arch/alpha/include/asm/cmpxchg.h
> > @@ -36,7 +36,6 @@
> > #undef ____cmpxchg
> > #define ____xchg(type, args...) __xchg ##type(args)
> > #define ____cmpxchg(type, args...) __cmpxchg ##type(args)
> > -#include <asm/xchg.h>
>
> It's amazing the number of times we get a patch to remove that.
>
> Instead of just automatically removing a second include of a
> header file, why don't you take a closer look to see what it
> actually does?
We run the static analyser "make includecheck" which list out files where
duplicate headers can be removed and based on that we thought to remove
from this file. Didn't understood about the existence of second include ??
On Fri, Nov 2, 2018 at 11:55 AM Souptick Joarder <[email protected]> wrote:
> We run the static analyser "make includecheck" which list out files where
> duplicate headers can be removed and based on that we thought to remove
> from this file. Didn't understood about the existence of second include ??
#define ____xchg(type, args...) __xchg ## type ## _local(args)
#define ____cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
#include <asm/xchg.h>
[snip]
#undef ____xchg
#undef ____cmpxchg
#define ____xchg(type, args...) __xchg ##type(args)
#define ____cmpxchg(type, args...) __cmpxchg ##type(args)
#include <asm/xchg.h>
asm/xchg.h has a comment at the top that says
/*
* xchg/xchg_local and cmpxchg/cmpxchg_local share the same code
* except that local version do not have the expensive memory barrier.
* So this file is included twice from asm/cmpxchg.h.
*/
On Sat, Nov 3, 2018 at 12:31 AM Matt Turner <[email protected]> wrote:
>
> On Fri, Nov 2, 2018 at 11:55 AM Souptick Joarder <[email protected]> wrote:
> > We run the static analyser "make includecheck" which list out files where
> > duplicate headers can be removed and based on that we thought to remove
> > from this file. Didn't understood about the existence of second include ??
>
>
> #define ____xchg(type, args...) __xchg ## type ## _local(args)
> #define ____cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
> #include <asm/xchg.h>
> [snip]
> #undef ____xchg
> #undef ____cmpxchg
> #define ____xchg(type, args...) __xchg ##type(args)
> #define ____cmpxchg(type, args...) __cmpxchg ##type(args)
> #include <asm/xchg.h>
>
> asm/xchg.h has a comment at the top that says
>
> /*
> * xchg/xchg_local and cmpxchg/cmpxchg_local share the same code
> * except that local version do not have the expensive memory barrier.
> * So this file is included twice from asm/cmpxchg.h.
> */
Thanks Matt. Sorry for the noise.
Is there any way to exclude it from static analyser that someone else will
not do the same mistake in future ?
On Fri, Nov 2, 2018 at 12:09 PM Souptick Joarder <[email protected]> wrote:
>
> On Sat, Nov 3, 2018 at 12:31 AM Matt Turner <[email protected]> wrote:
> >
> > On Fri, Nov 2, 2018 at 11:55 AM Souptick Joarder <[email protected]> wrote:
> > > We run the static analyser "make includecheck" which list out files where
> > > duplicate headers can be removed and based on that we thought to remove
> > > from this file. Didn't understood about the existence of second include ??
> >
> >
> > #define ____xchg(type, args...) __xchg ## type ## _local(args)
> > #define ____cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
> > #include <asm/xchg.h>
> > [snip]
> > #undef ____xchg
> > #undef ____cmpxchg
> > #define ____xchg(type, args...) __xchg ##type(args)
> > #define ____cmpxchg(type, args...) __cmpxchg ##type(args)
> > #include <asm/xchg.h>
> >
> > asm/xchg.h has a comment at the top that says
> >
> > /*
> > * xchg/xchg_local and cmpxchg/cmpxchg_local share the same code
> > * except that local version do not have the expensive memory barrier.
> > * So this file is included twice from asm/cmpxchg.h.
> > */
>
> Thanks Matt. Sorry for the noise.
> Is there any way to exclude it from static analyser that someone else will
> not do the same mistake in future ?
Since this is not an uncommon pattern in C, I think any static
analysis tool that attempts to find duplicate includes should attempt
to recognize such a pattern.
That, or humans should review the output of their static analysis
tools. Or you could try to compile the patches produced. I think any
of those would have caught the problem with the patch.
Hi Brajeswar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on sof-driver-fuweitax/master]
[also build test ERROR on v4.19 next-20181102]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Brajeswar-Ghosh/include-asm-cmpxchg-h-Remove-duplicate-header/20181103-072531
base: https://github.com/fuweitax/linux master
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=alpha
All errors (new ones prefixed by >>):
In file included from arch/alpha/include/asm/atomic.h:7:0,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/alpha/kernel/asm-offsets.c:10:
include/linux/atomic.h: In function 'atomic_inc_not_zero_hint':
arch/alpha/include/asm/cmpxchg.h:61:31: error: implicit declaration of function '__cmpxchg'; did you mean 'cmpxchg'? [-Werror=implicit-function-declaration]
__ret = (__typeof__(*(ptr))) __cmpxchg((ptr), \
^
arch/alpha/include/asm/atomic.h:205:38: note: in expansion of macro 'cmpxchg'
#define atomic_cmpxchg(v, old, new) (cmpxchg(&((v)->counter), old, new))
^~~~~~~
include/linux/atomic.h:596:9: note: in expansion of macro 'atomic_cmpxchg'
val = atomic_cmpxchg(v, c, c + 1);
^~~~~~~~~~~~~~
In file included from arch/alpha/include/asm/atomic.h:7:0,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/alpha/kernel/asm-offsets.c:10:
include/linux/debug_locks.h: In function '__debug_locks_off':
>> arch/alpha/include/asm/cmpxchg.h:50:3: error: implicit declaration of function '__xchg'; did you mean 'xchg'? [-Werror=implicit-function-declaration]
__xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
^
include/linux/debug_locks.h:17:9: note: in expansion of macro 'xchg'
return xchg(&debug_locks, 0);
^~~~
include/linux/llist.h: In function 'llist_del_all':
arch/alpha/include/asm/cmpxchg.h:49:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
__ret = (__typeof__(*(ptr))) \
^
include/linux/llist.h:234:9: note: in expansion of macro 'xchg'
return xchg(&head->first, NULL);
^~~~
cc1: some warnings being treated as errors
make[2]: *** [arch/alpha/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +50 arch/alpha/include/asm/cmpxchg.h
5ba840f9 Paul Gortmaker 2012-04-02 39
fbfcd019 Andrea Parri 2018-02-27 40 /*
fbfcd019 Andrea Parri 2018-02-27 41 * The leading and the trailing memory barriers guarantee that these
fbfcd019 Andrea Parri 2018-02-27 42 * operations are fully ordered.
fbfcd019 Andrea Parri 2018-02-27 43 */
5ba840f9 Paul Gortmaker 2012-04-02 44 #define xchg(ptr, x) \
5ba840f9 Paul Gortmaker 2012-04-02 45 ({ \
fbfcd019 Andrea Parri 2018-02-27 46 __typeof__(*(ptr)) __ret; \
5ba840f9 Paul Gortmaker 2012-04-02 47 __typeof__(*(ptr)) _x_ = (x); \
fbfcd019 Andrea Parri 2018-02-27 48 smp_mb(); \
fbfcd019 Andrea Parri 2018-02-27 49 __ret = (__typeof__(*(ptr))) \
fbfcd019 Andrea Parri 2018-02-27 @50 __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
fbfcd019 Andrea Parri 2018-02-27 51 smp_mb(); \
fbfcd019 Andrea Parri 2018-02-27 52 __ret; \
5ba840f9 Paul Gortmaker 2012-04-02 53 })
5ba840f9 Paul Gortmaker 2012-04-02 54
:::::: The code at line 50 was first introduced by commit
:::::: fbfcd0199170984bd3c2812e49ed0fe7b226959a locking/xchg/alpha: Remove superfluous memory barriers from the _local() variants
:::::: TO: Andrea Parri <[email protected]>
:::::: CC: Ingo Molnar <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Brajeswar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on sof-driver-fuweitax/master]
[also build test ERROR on v4.19 next-20181102]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Brajeswar-Ghosh/include-asm-cmpxchg-h-Remove-duplicate-header/20181103-072531
base: https://github.com/fuweitax/linux master
config: alpha-allmodconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=alpha
All error/warnings (new ones prefixed by >>):
In file included from arch/alpha/include/asm/atomic.h:7:0,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/alpha/kernel/asm-offsets.c:10:
include/linux/atomic.h: In function 'atomic_inc_not_zero_hint':
>> arch/alpha/include/asm/cmpxchg.h:61:31: error: implicit declaration of function '__cmpxchg'; did you mean 'cmpxchg'? [-Werror=implicit-function-declaration]
__ret = (__typeof__(*(ptr))) __cmpxchg((ptr), \
^
>> arch/alpha/include/asm/atomic.h:205:38: note: in expansion of macro 'cmpxchg'
#define atomic_cmpxchg(v, old, new) (cmpxchg(&((v)->counter), old, new))
^~~~~~~
>> include/linux/atomic.h:596:9: note: in expansion of macro 'atomic_cmpxchg'
val = atomic_cmpxchg(v, c, c + 1);
^~~~~~~~~~~~~~
In file included from arch/alpha/include/asm/atomic.h:7:0,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/alpha/kernel/asm-offsets.c:10:
include/linux/debug_locks.h: In function '__debug_locks_off':
>> arch/alpha/include/asm/cmpxchg.h:50:3: error: implicit declaration of function '__xchg'; did you mean '____xchg'? [-Werror=implicit-function-declaration]
__xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
^
>> include/linux/debug_locks.h:17:9: note: in expansion of macro 'xchg'
return xchg(&debug_locks, 0);
^~~~
include/linux/llist.h: In function 'llist_del_all':
>> arch/alpha/include/asm/cmpxchg.h:49:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
__ret = (__typeof__(*(ptr))) \
^
>> include/linux/llist.h:234:9: note: in expansion of macro 'xchg'
return xchg(&head->first, NULL);
^~~~
cc1: some warnings being treated as errors
make[2]: *** [arch/alpha/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +61 arch/alpha/include/asm/cmpxchg.h
5ba840f9 Paul Gortmaker 2012-04-02 39
fbfcd019 Andrea Parri 2018-02-27 40 /*
fbfcd019 Andrea Parri 2018-02-27 41 * The leading and the trailing memory barriers guarantee that these
fbfcd019 Andrea Parri 2018-02-27 42 * operations are fully ordered.
fbfcd019 Andrea Parri 2018-02-27 43 */
5ba840f9 Paul Gortmaker 2012-04-02 44 #define xchg(ptr, x) \
5ba840f9 Paul Gortmaker 2012-04-02 45 ({ \
fbfcd019 Andrea Parri 2018-02-27 46 __typeof__(*(ptr)) __ret; \
5ba840f9 Paul Gortmaker 2012-04-02 47 __typeof__(*(ptr)) _x_ = (x); \
fbfcd019 Andrea Parri 2018-02-27 48 smp_mb(); \
fbfcd019 Andrea Parri 2018-02-27 @49 __ret = (__typeof__(*(ptr))) \
fbfcd019 Andrea Parri 2018-02-27 @50 __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
fbfcd019 Andrea Parri 2018-02-27 51 smp_mb(); \
fbfcd019 Andrea Parri 2018-02-27 52 __ret; \
5ba840f9 Paul Gortmaker 2012-04-02 53 })
5ba840f9 Paul Gortmaker 2012-04-02 54
5ba840f9 Paul Gortmaker 2012-04-02 55 #define cmpxchg(ptr, o, n) \
5ba840f9 Paul Gortmaker 2012-04-02 56 ({ \
fbfcd019 Andrea Parri 2018-02-27 57 __typeof__(*(ptr)) __ret; \
5ba840f9 Paul Gortmaker 2012-04-02 58 __typeof__(*(ptr)) _o_ = (o); \
5ba840f9 Paul Gortmaker 2012-04-02 59 __typeof__(*(ptr)) _n_ = (n); \
fbfcd019 Andrea Parri 2018-02-27 60 smp_mb(); \
fbfcd019 Andrea Parri 2018-02-27 @61 __ret = (__typeof__(*(ptr))) __cmpxchg((ptr), \
fbfcd019 Andrea Parri 2018-02-27 62 (unsigned long)_o_, (unsigned long)_n_, sizeof(*(ptr)));\
fbfcd019 Andrea Parri 2018-02-27 63 smp_mb(); \
fbfcd019 Andrea Parri 2018-02-27 64 __ret; \
5ba840f9 Paul Gortmaker 2012-04-02 65 })
5ba840f9 Paul Gortmaker 2012-04-02 66
:::::: The code at line 61 was first introduced by commit
:::::: fbfcd0199170984bd3c2812e49ed0fe7b226959a locking/xchg/alpha: Remove superfluous memory barriers from the _local() variants
:::::: TO: Andrea Parri <[email protected]>
:::::: CC: Ingo Molnar <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation