powerpc/gamecube: make W=1 compilation errors free
Subsystems (2)
cover powerpc/gamecube: make W=1 compilation errors free
I started working on warnings treated as error on ppc32. As a first step, here is a set of patches to make gamecube error free when using W=1:
$ make ARCH=powerpc gamecube_defconfig $ make -j8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- W=1
Using:
$ powerpc-linux-gnu-gcc --version powerpc-linux-gnu-gcc (Debian 6.3.0-18) 6.3.0 20170516
Mathieu Malaterre (21):
powerpc: Remove warning on array size when empty
powerpc: Move the inline keyword at the beginning of function
declaration
powerpc: Mark the variable earlycon_acpi_spcr_enable maybe_unused
powerpc: Mark both tmp variables as unused
powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
powerpc: Make functions flipper_pic_init & ug_udbg_putc static
powerpc: Make function __giveup_fpu static
powerpc: Make function save_all static
powerpc: Add missing prototype for slb_miss_bad_addr
powerpc: Add missing prototype for hdec_interrupt
powerpc: Add missing prototype for time_init
powerpc: Add missing prototype for arch_dup_task_struct
powerpc: Add missing prototype for arch_irq_work_raise
powerpc: Add missing prototype for MMU_setup
powerpc: Add missing prototype for init_IRQ
powerpc: Add missing prototype for sys_debug_setcontext
powerpc: Add missing prototypes for sys_sigreturn & sys_rt_sigreturn
powerpc: Add missing prototypes for hw_breakpoint_handler &
arch_unregister_hw_breakpoint
powerpc: Add missing prototypes for ppc_select & ppc_fadvise64_64
powerpc: Add missing prototypes in setup_32.c
arch/powerpc/include/asm/asm-prototypes.h | 9 +++++++++ arch/powerpc/include/asm/hw_breakpoint.h | 2 ++ arch/powerpc/include/asm/irq.h | 1 + arch/powerpc/include/asm/irq_work.h | 1 + arch/powerpc/include/asm/page.h | 3 ++- arch/powerpc/include/asm/thread_info.h | 1 + arch/powerpc/include/asm/time.h | 2 ++ arch/powerpc/include/asm/uaccess.h | 2 +- arch/powerpc/kernel/process.c | 4 ++-- arch/powerpc/kernel/prom.c | 3 +-- arch/powerpc/kernel/setup.h | 5 +++++ arch/powerpc/kernel/setup_32.c | 2 ++ arch/powerpc/kernel/signal.h | 5 +++++ arch/powerpc/kernel/signal_32.c | 4 ++-- arch/powerpc/lib/sstep.c | 4 ++-- arch/powerpc/mm/init_32.c | 1 + arch/powerpc/platforms/embedded6xx/flipper-pic.c | 2 +- arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c | 2 +- include/linux/serial_core.h | 1 + 19 files changed, 42 insertions(+), 12 deletions(-)
Signature · 1 line
2.11.0
PATCH 1/21 powerpc: Remove warning on array size when empty
When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the array feature_properties is defined as an empty array, which in turn triggers the following warning (treated as error on W=1):
CC arch/powerpc/kernel/prom.oarch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
^cc1: all warnings being treated as errors
--- arch/powerpc/kernel/prom.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/powerpc/kernel/prom.c +1 -2
@@ -291,11 +291,10 @@ static inline void identical_pvr_fixup(unsigned long node)static void __init check_cpu_feature_properties (unsigned long node) { -unsigned long i;struct feature_property *fp = feature_properties;const __be32 *prop; -for (i =0 ; i <ARRAY_SIZE (feature_properties); ++i, ++fp) { +for (; fp != feature_properties +ARRAY_SIZE (feature_properties); ++fp) { prop =of_get_flat_dt_prop (node, fp->name,NULL );if (prop &&be32_to_cpup (prop) >= fp->min_value) { cur_cpu_spec->cpu_features |= fp->cpu_feature;
Signature · 1 line
2.11.0
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
Hi!
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned long node) · line 297
Quoted 6 lines · Mathieu Malaterre
When neither
CONFIG_ALTIVEC, norCONFIG_VSXorCONFIG_PPC64is defined, the arrayfeature_propertiesis defined as an empty array, which in turn triggers the following warning (treated as error on W=1):arch/powerpc/kernel/prom.c +1
-1-for (i =0 ; i <ARRAY_SIZE (feature_properties); ++i, ++fp) { +for (; fp != feature_properties +ARRAY_SIZE (feature_properties); ++fp) {
You could just write != instead of < ? Seems more readable.
Maybe something can be done to ARRAY_SIZE to make this not warn.
Segher
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
In reply to Mathieu MalaterreOn Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <[email protected]> wrote= :
Quoted 1 line
When neither
CONFIG_ALTIVEC, norCONFIG_VSXorCONFIG_PPC64is defined, t=
he
Quoted 5 lines
array
feature_propertiesis defined as an empty array, which in turn triggers the following warning (treated as error on W=3D1):CC
arch/powerpc/kernel/prom.oarch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_proper=
ties=E2=80=99:
Quoted 1 line
arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expressi=
on < 0 is always false [-Werror=3Dtype-limits]
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned lon=
Quoted 14 lines
for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) { ^cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/kernel/prom.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/powerpc/kernel/prom.c +0
-0@@ -291,11 +291,10 @@ static inline void identical_pvr_fixup(unsigned lon=
g node)
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned long node) · line 296
Quoted 7 lines
arch/powerpc/kernel/prom.c +0
-1static void __init check_cpu_feature_properties (unsigned long node) { -unsigned long i; struct feature_property *fp =3D feature_properties;const __be32 *prop;
Much simpler is just add
if (ARRAY_SIZE() =3D=3D 0)
return;
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned lon= · line 296
Quoted 2 lines
arch/powerpc/kernel/prom.c +1
-1- for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) { + for (; fp !=3D feature_properties + ARRAY_SIZE(feature_properties=
); ++fp) {
Quoted 6 lines
arch/powerpc/kernel/prom.c +0
-1prop =3D of_get_flat_dt_prop(node, fp->name, NULL); if (prop && be32_to_cpup(prop) >=3D fp->min_value) { cur_cpu_spec->cpu_features |=3D fp->cpu_feature; --2.11.0
--=20
With Best Regards,
Andy Shevchenko
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
Quoted 9 lines · Andy Shevchenko
Quoted 6 lines · Mathieu Malaterre
static void
__initcheck_cpu_feature_properties(unsigned long node){- unsigned long i; struct feature_property *fp = feature_properties; const __be32 *prop;
Quoted 7 lines
Much simpler is just add
if (
ARRAY_SIZE()== 0)
return;Quoted 2 lines
arch/powerpc/kernel/prom.c +1
-1-for (i =0 ; i <ARRAY_SIZE (feature_properties); ++i, ++fp) { +for (; fp != feature_properties +ARRAY_SIZE (feature_properties); ++fp) {
...or convert to while(), which will be more readable.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
Quoted 13 lines · Andy Shevchenko
Quoted 11 lines · Andy Shevchenko
Quoted 6 lines · Mathieu Malaterre
static void
__initcheck_cpu_feature_properties(unsigned long node){- unsigned long i; struct feature_property *fp = feature_properties; const __be32 *prop;Much simpler is just add
Quoted 7 lines
Quoted 5 lines
if (
ARRAY_SIZE()== 0)
return;Quoted 2 lines
arch/powerpc/kernel/prom.c +1
-1-for (i =0 ; i <ARRAY_SIZE (feature_properties); ++i, ++fp) { +for (; fp != feature_properties +ARRAY_SIZE (feature_properties); ++fp) {...or convert to
while(), which will be more readable.
So you'd prefer something like:
while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
...
++fp;
}
right ?
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
Le 27/02/2018 à 08:25, Mathieu Malaterre a écrit :
Quoted 23 lines
Quoted 13 lines · Andy Shevchenko
Quoted 11 lines · Andy Shevchenko
Quoted 6 lines · Mathieu Malaterre
static void
__initcheck_cpu_feature_properties(unsigned long node){- unsigned long i; struct feature_property *fp = feature_properties; const __be32 *prop;Much simpler is just add
Quoted 7 lines
Quoted 5 lines
if (
ARRAY_SIZE()== 0)
return;Quoted 2 lines
arch/powerpc/kernel/prom.c +1
-1-for (i =0 ; i <ARRAY_SIZE (feature_properties); ++i, ++fp) { +for (; fp != feature_properties +ARRAY_SIZE (feature_properties); ++fp) {...or convert to
while(), which will be more readable.
Quoted 9 lines
So you'd prefer something like:
while (fp < feature_properties + ARRAY_SIZE(feature_properties)) { ... ++fp; }right ?
Why not do as suggested by Segher, ie just replace < by != in the original form ? Or add in front:
if (!ARRAY_SIZE(feature_properties))
return;
Christophe
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
Quoted 27 lines · Christophe LEROY
Le 27/02/2018 =C3=A0 08:25, Mathieu Malaterre a =C3=A9crit :
Quoted 24 lines
Quoted 21 lines · Andy Shevchenko
Quoted 18 lines · Andy Shevchenko
Quoted 6 lines · Mathieu Malaterre
static void
__initcheck_cpu_feature_properties(unsigned long node){- unsigned long i; struct feature_property *fp =3D feature_properties; const __be32 *prop;Much simpler is just add
if (
ARRAY_SIZE()=3D=3D 0)
return;Quoted 1 line
- for (i =3D 0; i <
ARRAY_SIZE(feature_properties); ++i, ++fp) =
{
Quoted 14 lines
Quoted 14 lines
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
+ for (; fp !=3D
feature_properties+ARRAY_SIZE(feature_properties); ++fp) {...or convert to
while(), which will be more readable.So you'd prefer something like:
while (fp < feature_properties + ARRAY_SIZE(feature_properties)) { ... ++fp; }
Quoted 5 lines
Quoted 2 lines
right ?
Why not do as suggested by Segher, ie just replace < by !=3D in the origi=
nal
Quoted 1 line
form ?
I can do that.
Quoted 3 lines
Or add in front:
if (!ARRAY_SIZE(feature_properties))return;
(not tested) I believe the compiler still go over the for() loop and will complain about the original unsigned comparison.
Quoted 1 line
Christophe
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
Quoted 8 lines · Mathieu Malaterre
Quoted 4 lines · Christophe LEROY
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Much simpler is just add
if (
ARRAY_SIZE()== 0)
return;
Quoted 6 lines
Quoted 3 lines
Or add in front:
if (!ARRAY_SIZE(feature_properties))return;(not tested) I believe the compiler still go over the
for()loop and will complain about the original unsigned comparison.
Did you run tests? Did you look into object file?
In kernel we much rely on the compiling away the code which is deterministically not in use.
Here I'm pretty sure it will compile away entire function.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
Quoted 1 line · Andy Shevchenko
On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre <[email protected]> wro=
te:
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines · Christophe LEROY
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Much simpler is just add
if (
ARRAY_SIZE()=3D=3D 0)
return;
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Or add in front:
if (!ARRAY_SIZE(feature_properties))return;(not tested) I believe the compiler still go over the
for()loop and will complain about the original unsigned comparison.Did you run tests? Did you look into object file?
The goal of this series is simply to remove warning treated as error.
I tried from home and I can still see the error using gcc 6.3.0, so the original for() loop needs to be rewritten.
CC arch/powerpc/kernel/prom.oarch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_properti= es=E2=80=99:arch/powerpc/kernel/prom.c:301:16: error: comparison of unsigned expression < 0 is always false [-Werror=3Dtype-limits]
for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
^cc1: all warnings being treated as errors
Quoted 7 lines
In kernel we much rely on the compiling away the code which is deterministically not in use.
Here I'm pretty sure it will compile away entire function.--
With Best Regards,
Andy Shevchenko
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
In reply to Andy ShevchenkoQuoted 13 lines · Andy Shevchenko
Quoted 12 lines · Mathieu Malaterre
Quoted 8 lines · Christophe LEROY
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Much simpler is just add
if (
ARRAY_SIZE()== 0)
return;Or add in front:
if (!ARRAY_SIZE(feature_properties))return;
Quoted 8 lines
Quoted 2 lines
(not tested) I believe the compiler still go over the
for()loop and will complain about the original unsigned comparison.Did you run tests? Did you look into object file?
In kernel we much rely on the compiling away the code which is deterministically not in use.
Here I'm pretty sure it will compile away entire function.
It does, but it also still warns (this warning is done very early in the compiler pipeline).
Segher
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
Quoted 17 lines · Segher Boessenkool
Quoted 16 lines · Andy Shevchenko
Quoted 14 lines · Mathieu Malaterre
Quoted 8 lines · Christophe LEROY
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Much simpler is just add
if (
ARRAY_SIZE()== 0)
return;Or add in front:
if (!ARRAY_SIZE(feature_properties))return;(not tested) I believe the compiler still go over the
for()loop and will complain about the original unsigned comparison.
Quoted 8 lines
Quoted 5 lines
Did you run tests? Did you look into object file?
In kernel we much rely on the compiling away the code which is deterministically not in use.
Here I'm pretty sure it will compile away entire function.It does, but it also still warns (this warning is done very early in the compiler pipeline).
Oh, I see.
Then the while () approach looks to me the best here.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Michael Ellerman
reviewerRe: [PATCH 01/21] powerpc: Remove warning on array size when empty
In reply to Mathieu MalaterreQuoted 1 line · Mathieu Malaterre
When neither
CONFIG_ALTIVEC, norCONFIG_VSXorCONFIG_PPC64is defined, t=
he
Quoted 5 lines
array
feature_propertiesis defined as an empty array, which in turn triggers the following warning (treated as error on W=3D1):CC
arch/powerpc/kernel/prom.oarch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_proper=
ties=E2=80=99:
Quoted 1 line
arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expressi=
on < 0 is always false [-Werror=3Dtype-limits]
Quoted 3 lines
for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) { ^cc1: all warnings being treated as errors
Ugh, that's annoying.
This seems to work?
arch/powerpc/kernel/prom.c +0 -0
@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long =
node)
=20
static void __init check_cpu_feature_properties(unsigned long node) {
- unsigned long i;
struct feature_property *fp =3D feature_properties; const __be32 *prop;
+ int i;
=20
- for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+ for (i =3D 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
prop =3D of_get_flat_dt_prop(node, fp->name, NULL); if (prop && be32_to_cpup(prop) >=3D fp->min_value) {
cur_cpu_spec->cpu_features |=3D fp->cpu_feature;
cheers
Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman <[email protected]> wrote= :
Quoted 3 lines
Quoted 1 line · Mathieu Malaterre
When neither
CONFIG_ALTIVEC, norCONFIG_VSXorCONFIG_PPC64is defined, =
the
Quoted 5 lines
Quoted 5 lines
array
feature_propertiesis defined as an empty array, which in turn triggers the following warning (treated as error on W=3D1):CC
arch/powerpc/kernel/prom.oarch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_prope=
rties=E2=80=99:
Quoted 1 line
Quoted 1 line
arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned express=
ion < 0 is always false [-Werror=3Dtype-limits]
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned lon=
Quoted 13 lines
Quoted 3 lines
for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) { ^cc1: all warnings being treated as errors
Ugh, that's annoying.
This seems to work?
arch/powerpc/kernel/prom.c +0
-0@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned lon=
g node)
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned lon= · line 297
Quoted 10 lines
arch/powerpc/kernel/prom.c +2
-2static void __init check_cpu_feature_properties (unsigned long node) { -unsigned long i; struct feature_property *fp =3D feature_properties;const __be32 *prop; +int i; - for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) { + for (i =3D 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp)=
{
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned lon= · line 301
Quoted 4 lines
arch/powerpc/kernel/prom.c +0
-0prop =3D of_get_flat_dt_prop(node, fp->name, NULL); if (prop && be32_to_cpup(prop) >=3D fp->min_value) { cur_cpu_spec->cpu_features |=3D fp->cpu_feature;
Indeed that looks like the less invasive solution, I'll re-submit.
Should I resubmit the entire patch series (21 indep patches) or re-submit only the 3 patches that were discussed (as part of a different series) ?
Thanks
Michael Ellerman
reviewerRe: [PATCH 01/21] powerpc: Remove warning on array size when empty
Quoted 1 line · Mathieu Malaterre
On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman <[email protected]> wro=
te:
Quoted 3 lines
Quoted 3 lines
Quoted 1 line · Mathieu Malaterre
When neither
CONFIG_ALTIVEC, norCONFIG_VSXorCONFIG_PPC64is defined,=
the
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
array
feature_propertiesis defined as an empty array, which in turn triggers the following warning (treated as error on W=3D1):CC
arch/powerpc/kernel/prom.oarch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_prop=
erties=E2=80=99:
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expres=
sion < 0 is always false [-Werror=3Dtype-limits]
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) { ^cc1: all warnings being treated as errors
Ugh, that's annoying.
Quoted 7 lines
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned lo=
Quoted 7 lines
This seems to work?
arch/powerpc/kernel/prom.c +0
-0@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned lo=
ng node)
Quoted 10 lines
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned lo= · line 297
Quoted 10 lines
arch/powerpc/kernel/prom.c +2
-2static void __init check_cpu_feature_properties(unsigned long node) { -unsigned long i; struct feature_property *fp =3D feature_properties; const __be32 *prop; + int i; - for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) { + for (i =3D 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp=
) {
Quoted 6 lines
arch/powerpc/kernel/prom.c static inline void identical_pvr_fixup(unsigned lo= · line 301
Quoted 4 lines
arch/powerpc/kernel/prom.c +0
-0prop =3D of_get_flat_dt_prop(node, fp->name, NULL); if (prop && be32_to_cpup(prop) >=3D fp->min_value) { cur_cpu_spec->cpu_features |=3D fp->cpu_feature;Indeed that looks like the less invasive solution, I'll re-submit.
Thanks.
Quoted 3 lines
Should I resubmit the entire patch series (21 indep patches) or re-submit only the 3 patches that were discussed (as part of a different series) ?
Just resubmit the ones that need changes. You can either send them as a new series of 3, or post each as a reply to the original patch with v2 in the subject.
cheers
PATCH v2 1/21 powerpc: Remove warning on array size when empty
In reply to Mathieu MalaterreWhen neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the array feature_properties is defined as an empty array, which in turn triggers the following warning (treated as error on W=1):
CC arch/powerpc/kernel/prom.oarch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
^cc1: all warnings being treated as errors
--- arch/powerpc/kernel/prom.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/powerpc/kernel/prom.c +2 -2
@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long node)static void __init check_cpu_feature_properties (unsigned long node) { -unsigned long i; +int i;struct feature_property *fp = feature_properties;const __be32 *prop; -for (i =0 ; i <ARRAY_SIZE (feature_properties); ++i, ++fp) { +for (i =0 ; i < (int )ARRAY_SIZE (feature_properties); ++i, ++fp) { prop =of_get_flat_dt_prop (node, fp->name,NULL );if (prop &&be32_to_cpup (prop) >= fp->min_value) { cur_cpu_spec->cpu_features |= fp->cpu_feature;
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [v2,01/21] powerpc: Remove warning on array size when empty
Quoted 7 lines · Mathieu Malaterre
When neither
CONFIG_ALTIVEC, norCONFIG_VSXorCONFIG_PPC64is defined, the arrayfeature_propertiesis defined as an empty array, which in turn triggers the following warning (treated as error on W=1):CC
arch/powerpc/kernel/prom.oarch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
Quoted 6 lines
for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) { ^cc1: all warnings being treated as errors
Suggested-by Michael Ellerman <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/4f1f40f7b2b4487f582ecafec64076
cheers
PATCH 2/21 powerpc: Move the inline keyword at the beginning of function declaration
In reply to Mathieu MalaterreThe inline keyword was not at the beginning of the function declaration. Fix the following warning (treated as error in W=1)
CC kernel/sys.oarch/powerpc/lib/sstep.c:283:1: error: ‘inline’ is not at beginning of declaration [-Werror=old-style-declaration]
static int nokprobe_inline copy_mem_in(u8 *dest, unsigned long ea, int nb,
^~~~~~arch/powerpc/lib/sstep.c:388:1: error: ‘inline’ is not at beginning of declaration [-Werror=old-style-declaration]
static int nokprobe_inline copy_mem_out(u8 *dest, unsigned long ea, int nb,
^~~~~~
CC arch/powerpc/kernel/setup_32.o
--- arch/powerpc/lib/sstep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/powerpc/lib/sstep.c +2 -2
@@ -280,7 +280,7 @@ static nokprobe_inline int read_mem_aligned(unsigned long *dest,* Copy from userspace to a buffer, using the largest possible * aligned accesses, up to sizeof(long). */ -static int nokprobe_inline copy_mem_in(u8 *dest, unsigned long ea, int nb, +static nokprobe_inline int copy_mem_in(u8 *dest, unsigned long ea, int nb,struct pt_regs *regs) {int err =0 ;
@@ -385,7 +385,7 @@ static nokprobe_inline int write_mem_aligned(unsigned long val,* Copy from a buffer to userspace, using the largest possible * aligned accesses, up to sizeof(long). */ -static int nokprobe_inline copy_mem_out(u8 *dest, unsigned long ea, int nb, +static nokprobe_inline int copy_mem_out(u8 *dest, unsigned long ea, int nb,struct pt_regs *regs) {int err =0 ;
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [02/21] powerpc: Move the inline keyword at the beginning of function declaration
Quoted 12 lines · Mathieu Malaterre
The inline keyword was not at the beginning of the function declaration. Fix the following warning (treated as error in W=1)
CC
kernel/sys.oarch/powerpc/lib/sstep.c:283:1: error: ‘inline’ is not at beginning of declaration [-Werror=old-style-declaration]
static intnokprobe_inlinecopy_mem_in(u8 *dest, unsigned long ea, int nb,
^~~~~~arch/powerpc/lib/sstep.c:388:1: error: ‘inline’ is not at beginning of declaration [-Werror=old-style-declaration]
static intnokprobe_inlinecopy_mem_out(u8 *dest, unsigned long ea, int nb,
^~~~~~
CCarch/powerpc/kernel/setup_32.o
Quoted 1 line
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/174b701d3dcd14514f869e2bc08ac4
cheers
PATCH 3/21 powerpc: Mark the variable earlycon_acpi_spcr_enable maybe_unused
In reply to Mathieu MalaterreRe-use the object-like macro EARLYCON_USED_OR_UNUSED to mark earlycon_acpi_spcr_enable as maybe_unused.
Fix the following warning (treated as error in W=1)
CC arch/powerpc/kernel/setup-common.o
In file included from ./include/linux/serial_8250.h:14:0,
from arch/powerpc/kernel/setup-common.c:33:./include/linux/serial_core.h:382:19: error: ‘earlycon_acpi_spcr_enable’ defined but not used [-Werror=unused-const-variable=]
static const bool earlycon_acpi_spcr_enable;
^~~~~~~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
--- include/linux/serial_core.h | 1 + 1 file changed, 1 insertion(+)
include/linux/serial_core.h +1 -0
@@ -379,6 +379,7 @@ extern int of_setup_earlycon(const struct earlycon_id *match,extern bool earlycon_acpi_spcr_enable__initdata ;int setup_earlycon (char *buf);#else +EARLYCON_USED_OR_UNUSED static const bool earlycon_acpi_spcr_enable;static inline int setup_earlycon (char *buf) {return 0 ; }#endif
Signature · 1 line
2.11.0
Michael Ellerman
reviewerRe: [PATCH 03/21] powerpc: Mark the variable earlycon_acpi_spcr_enable maybe_unused
Quoted 9 lines · Mathieu Malaterre
Re-use the object-like macro
EARLYCON_USED_OR_UNUSEDto markearlycon_acpi_spcr_enableasmaybe_unused.Fix the following warning (treated as error in W=3D1)
CC
arch/powerpc/kernel/setup-common.o
In file included from./include/linux/serial_8250.h:14:0,
fromarch/powerpc/kernel/setup-common.c:33:./include/linux/serial_core.h:382:19: error: =E2=80=98earlycon_acpi_spcr_=
enable=E2=80=99 defined but not used [-Werror=3Dunused-const-variable=3D]
Quoted 7 lines
static const bool earlycon_acpi_spcr_enable; ^~~~~~~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>--- include/linux/serial_core.h | 1 +
I can't take this one as that's not a file I maintain.
The script says:
$ ./scripts/get_maintainer.pl include/linux/serial_core.h [email protected] [email protected] [email protected]
Can you resend it to them?
include/linux/serial_core.h extern int of_setup_earlycon(const struct earlycon_id=
Quoted 5 lines
include/linux/serial_core.h +0
-0@@ -379,6 +379,7 @@ extern int of_setup_earlycon(const struct earlycon_id=
*match,
include/linux/serial_core.h extern int of_setup_earlycon(const struct earlycon_id *match, · line 383
Quoted 5 lines
include/linux/serial_core.h +1
-0extern bool earlycon_acpi_spcr_enable__initdata ;int setup_earlycon (char *buf);#else +EARLYCON_USED_OR_UNUSED static const bool earlycon_acpi_spcr_enable;
The macro eventually turns into an __attribute__, which I think is typically placed after the variable, so eg:
static const bool earlycon_acpi_spcr_enable EARLYCON_USED_OR_UNUSED;
cheers
Quoted 4 lines
include/linux/serial_core.h +0
-1static inline int setup_earlycon (char *buf) {return 0 ; }#endif --=202.11.0
Re: [PATCH 03/21] powerpc: Mark the variable earlycon_acpi_spcr_enable maybe_unused
On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <[email protected]> wrot= e:
Quoted 11 lines
Quoted 9 lines · Mathieu Malaterre
Re-use the object-like macro
EARLYCON_USED_OR_UNUSEDto markearlycon_acpi_spcr_enableasmaybe_unused.Fix the following warning (treated as error in W=3D1)
CC
arch/powerpc/kernel/setup-common.o
In file included from./include/linux/serial_8250.h:14:0,
fromarch/powerpc/kernel/setup-common.c:33:./include/linux/serial_core.h:382:19: error: =E2=80=98earlycon_acpi_spcr=
_enable=E2=80=99 defined but not used [-Werror=3Dunused-const-variable=3D]
Quoted 12 lines
Quoted 7 lines
static const bool earlycon_acpi_spcr_enable; ^~~~~~~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>--- include/linux/serial_core.h | 1 +I can't take this one as that's not a file I maintain.
The script says:
Quoted 7 lines
$ ./scripts/get_maintainer.pl include/linux/serial_core.h [email protected] [email protected] [email protected]Can you resend it to them?
Ah right, thanks.
Quoted 6 lines
include/linux/serial_core.h extern int of_setup_earlycon(const struct earlycon_i=
Quoted 6 lines
include/linux/serial_core.h +0
-0@@ -379,6 +379,7 @@ extern int of_setup_earlycon(const struct earlycon_i=
d *match,
Quoted 11 lines
include/linux/serial_core.h extern int of_setup_earlycon(const struct earlycon_id *match, · line 383
Quoted 5 lines
include/linux/serial_core.h +1
-0extern bool earlycon_acpi_spcr_enable__initdata ;int setup_earlycon (char *buf);#else +EARLYCON_USED_OR_UNUSED static const bool earlycon_acpi_spcr_enable;The macro eventually turns into an
__attribute__, which I think is typically placed after the variable, so eg:static const bool
earlycon_acpi_spcr_enableEARLYCON_USED_OR_UNUSED;
Indeed makes it more consistent with style. Thanks!
Quoted 6 lines
cheers
Quoted 4 lines
include/linux/serial_core.h +0
-1static inline int setup_earlycon (char *buf) {return 0 ; }#endif --2.11.0
PATCH 4/21 powerpc: Mark both tmp variables as unused
In reply to Mathieu MalaterreSince the value of tmp is never intended to be read, declare both tmp variables as unused. Fix warning (treated as error in W=1):
CC arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c: In function ‘sys_swapcontext’:arch/powerpc/kernel/signal_32.c:1048:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]
unsigned char tmp;
^~~
arch/powerpc/kernel/signal_32.c: In function ‘sys_debug_setcontext’:
arch/powerpc/kernel/signal_32.c:1234:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]
unsigned char tmp;
^~~cc1: all warnings being treated as errors
--- arch/powerpc/kernel/signal_32.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/powerpc/kernel/signal_32.c +2 -2
@@ -1045,7 +1045,7 @@ long sys_swapcontext(struct ucontext __user *old_ctx,struct ucontext __user *new_ctx, int ctx_size, int r6, int r7, int r8, struct pt_regs *regs) { - unsigned char tmp; + unsigned char tmp __maybe_unused;int ctx_has_vsx_region =0 ;#ifdef CONFIG_PPC64
@@ -1231,7 +1231,7 @@ int sys_debug_setcontext(struct ucontext __user *ctx, {struct sig_dbg_op op;int i; -unsigned char tmp; +unsigned char tmp__maybe_unused ;unsigned long new_msr = regs->msr;#ifdef CONFIG_PPC_ADV_DEBUG_REGS unsigned long new_dbcr0 = current->thread.debug.dbcr0;
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [04/21] powerpc: Mark both tmp variables as unused
Quoted 12 lines · Mathieu Malaterre
Since the value of
tmpis never intended to be read, declare bothtmpvariables as unused. Fix warning (treated as error in W=1):CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c: In function ‘sys_swapcontext’:arch/powerpc/kernel/signal_32.c:1048:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]unsigned char tmp; ^~~ arch/powerpc/kernel/signal_32.c: In function ‘sys_debug_setcontext’: arch/powerpc/kernel/signal_32.c:1234:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable] unsigned char tmp; ^~~
Quoted 3 lines
cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/67b464a89c21c9edd45ad15c457bb5
cheers
Re: [PATCH 04/21] powerpc: Mark both tmp variables as unused
In reply to Mathieu MalaterreLe 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
Quoted 2 lines
Since the value of
tmpis never intended to be read, declare bothtmpvariables as unused. Fix warning (treated as error in W=1):
What about using fault_in_pages_readable() instead ?
Christophe
Quoted 40 lines
CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c: In function ‘sys_swapcontext’:arch/powerpc/kernel/signal_32.c:1048:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]unsigned char tmp; ^~~ arch/powerpc/kernel/signal_32.c: In function ‘sys_debug_setcontext’: arch/powerpc/kernel/signal_32.c:1234:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable] unsigned char tmp; ^~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/kernel/signal_32.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/powerpc/kernel/signal_32.c +2
-2@@ -1045,7 +1045,7 @@ long sys_swapcontext(struct ucontext __user *old_ctx,struct ucontext __user *new_ctx, int ctx_size, int r6, int r7, int r8, struct pt_regs *regs) { - unsigned char tmp; + unsigned char tmp __maybe_unused;int ctx_has_vsx_region =0 ;#ifdef CONFIG_PPC64 @@ -1231,7 +1231,7 @@ int sys_debug_setcontext(struct ucontext __user *ctx, {struct sig_dbg_op op;int i; -unsigned char tmp; +unsigned char tmp__maybe_unused ;unsigned long new_msr = regs->msr;#ifdef CONFIG_PPC_ADV_DEBUG_REGS unsigned long new_dbcr0 = current->thread.debug.dbcr0;
Michael Ellerman
reviewerRe: [PATCH 04/21] powerpc: Mark both tmp variables as unused
Quoted 5 lines · Christophe LEROY
Le 25/02/2018 =C3=A0 18:22, Mathieu Malaterre a =C3=A9crit=C2=A0:
Quoted 2 lines
Since the value of
tmpis never intended to be read, declare bothtmpvariables as unused. Fix warning (treated as error in W=3D1):What about using
fault_in_pages_readable()instead ?
Yeah that looks like it would work.
I'd be happy to take a tested patch :D
cheers
PATCH 5/21 powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
In reply to Mathieu MalaterreRewrite comparison since all values compared are of type unsigned long.
Fix a warning (treated as error in W=1):
CC arch/powerpc/kernel/irq.o
In file included from ./include/linux/bug.h:5:0,
from ./include/linux/cpumask.h:13,
from ./include/linux/smp.h:13,
from ./include/linux/kernel_stat.h:5,
from arch/powerpc/kernel/irq.c:35:
./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
#define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
^--- arch/powerpc/include/asm/page.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/powerpc/include/asm/page.h +2 -1
@@ -126,7 +126,8 @@ extern long long virt_phys_offset;#ifdef CONFIG_FLATMEM #define ARCH_PFN_OFFSET ((unsigned long)(MEMORY_START >> PAGE_SHIFT)) -#define pfn_valid (pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr) +#define pfn_valid (pfn) \ + (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))#endif #define virt_to_pfn (kaddr) (__pa(kaddr) >> PAGE_SHIFT)
Signature · 1 line
2.11.0
Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
Quoted 25 lines
Rewrite comparison since all values compared are of type
unsigned long.Fix a warning (treated as error in W=1):
CC
arch/powerpc/kernel/irq.o
In file included from./include/linux/bug.h:5:0,from ./include/linux/cpumask.h:13, from ./include/linux/smp.h:13, from ./include/linux/kernel_stat.h:5, from arch/powerpc/kernel/irq.c:35: ./include/linux/dma-mapping.h: In function ‘dma_map_resource’: ./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits] #define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr) ^Suggested-by Segher Boessenkool <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/include/asm/page.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/powerpc/include/asm/page.h +0
-0
arch/powerpc/include/asm/page.h extern long long virt_phys_offset; · line 130
Quoted 6 lines
arch/powerpc/include/asm/page.h +2
-1@@ -126,7 +126,8 @@ extern long long virt_phys_offset;#ifdef CONFIG_FLATMEM #define ARCH_PFN_OFFSET ((unsigned long)(MEMORY_START >> PAGE_SHIFT)) -#define pfn_valid (pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr) +#define pfn_valid (pfn) \ + (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
What will happen when ARCH_PFN_OFFSET is not nul and pfn is lower than ARCH_PFN_OFFSET ?
Christophe
Quoted 4 lines
arch/powerpc/include/asm/page.h +0
-0#endif #define virt_to_pfn (kaddr) (__pa(kaddr) >> PAGE_SHIFT)
Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
Quoted 5 lines · Christophe LEROY
Le 25/02/2018 =C3=A0 18:22, Mathieu Malaterre a =C3=A9crit :
Quoted 2 lines
Rewrite comparison since all values compared are of type
unsigned long=
.
Quoted 10 lines
Quoted 10 lines
Fix a warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/irq.o
In file included from./include/linux/bug.h:5:0,from ./include/linux/cpumask.h:13, from ./include/linux/smp.h:13, from ./include/linux/kernel_stat.h:5, from arch/powerpc/kernel/irq.c:35:
./include/linux/dma-mapping.h: In function =E2=80=98dma_map_resource=E2=
=80=99:
Quoted 3 lines
Quoted 3 lines
./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >=3D 0 is always true [-Werror=3Dtype-limits]
#definepfn_valid(pfn)((pfn) >=3DARCH_PFN_OFFSET&& (pfn) < max_map=
nr)
Quoted 17 lines
Quoted 17 lines
^Suggested-by Segher Boessenkool <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/include/asm/page.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/powerpc/include/asm/page.h +0
-1@@ -126,7 +126,8 @@ extern long long virt_phys_offset;#ifdef CONFIG_FLATMEM #define ARCH_PFN_OFFSET ((unsigned long)(MEMORY_START >> PAGE_SHIFT)) -#define pfn_valid (pfn) ((pfn) >=3D ARCH_PFN_OFFSET && (pfn) <
Quoted 8 lines
arch/powerpc/include/asm/page.h extern long long virt_phys_offset; · line 132
Quoted 4 lines
arch/powerpc/include/asm/page.h +2
-0max_mapnr) +#define pfn_valid (pfn) \ + (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))What will happen when
ARCH_PFN_OFFSETis not nul and pfn is lower thanARCH_PFN_OFFSET?
I assumed that normal unsigned integers modulo would make the test fail. But for some particular value of max_mapnr the two are indeed not equivalent.
Quoted 7 lines
Christophe
Quoted 3 lines
arch/powerpc/include/asm/page.h +0
-0#endif #define virt_to_pfn(kaddr) (__pa(kaddr) >> PAGE_SHIFT)
Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
In reply to Christophe LEROYQuoted 8 lines · Christophe LEROY
Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
arch/powerpc/include/asm/page.h extern long long virt_phys_offset; · line 130
Quoted 4 lines
-#define
pfn_valid(pfn)((pfn) >=ARCH_PFN_OFFSET&& (pfn) <max_mapnr)arch/powerpc/include/asm/page.h +2
-0+#define pfn_valid (pfn) \ + (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))What will happen when
ARCH_PFN_OFFSETis not nul and pfn is lower thanARCH_PFN_OFFSET?
It will work fine.
Say you are asking for a <= x < b so (in actual integers, no overflow) that is 0 <= x-a < b-a and you also assume x-a overflows, so that we are actually comparing x-a+M < b-a with M = 2**32 or such (the maximum value in the unsigned integer type plus one). This comparison is obviously always false.
(It also works if b < a btw).
Segher
Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
Quoted 3 lines · Segher Boessenkool
Quoted 2 lines · Christophe LEROY
Le 25/02/2018 =C3=A0 18:22, Mathieu Malaterre a =C3=A9crit :
Quoted 1 line
-#define
pfn_valid(pfn)((pfn) >=3DARCH_PFN_OFFSET&& (pf=
n) <
Quoted 3 lines
Quoted 3 lines
Review
Quoted 3 lines
max_mapnr)(quoted diff) +2
-0+#define pfn_valid(pfn) \ + (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET)=
)
Quoted 7 lines
Quoted 3 lines
What will happen when
ARCH_PFN_OFFSETis not nul and pfn is lower thanARCH_PFN_OFFSET?It will work fine.
Say you are asking for a <=3D x < b so (in actual integers, no overflow=
)
Quoted 2 lines
that is 0 <=3D x-a < b-a and you also assume x-a overflows, so that we are actually comparing x-a+M < b-a with M =3D 2**32 or such (the maximu=
m
Quoted 6 lines
value in the unsigned integer type plus one). This comparison is obviously always false.
(It also works if b < a btw).
Thanks Segher !
Christophe does that clarify things or do you want me to update the commit message ?
christophe leroy
Reviewed-byRe: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
Le 02/03/2018 à 20:54, Mathieu Malaterre a écrit :
Quoted 20 lines
Quoted 18 lines · Segher Boessenkool
Quoted 8 lines · Christophe LEROY
arch/powerpc/include/asm/page.h extern long long virt_phys_offset; · line 130
Quoted 4 lines · Mathieu Malaterre
-#define
pfn_valid(pfn)((pfn) >=ARCH_PFN_OFFSET&& (pfn) <max_mapnr)arch/powerpc/include/asm/page.h +2
-0+#define pfn_valid (pfn) \ + (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))What will happen when
ARCH_PFN_OFFSETis not nul and pfn is lower thanARCH_PFN_OFFSET?It will work fine.
Say you are asking for a <= x < b so (in actual integers, no overflow) that is 0 <= x-a < b-a and you also assume x-a overflows, so that we are actually comparing x-a+M < b-a with M = 2**32 or such (the maximum value in the unsigned integer type plus one). This comparison is obviously always false.
Quoted 8 lines
Quoted 3 lines
(It also works if b < a btw).
Thanks Segher !
Christophe does that clarify things or do you want me to update the commit message ?
No it is fine for me.
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
Michael Ellerman
reviewerRe: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
In reply to Mathieu MalaterreQuoted 11 lines · Mathieu Malaterre
Rewrite comparison since all values compared are of type
unsigned long.Fix a warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/irq.o
In file included from./include/linux/bug.h:5:0,from ./include/linux/cpumask.h:13, from ./include/linux/smp.h:13, from ./include/linux/kernel_stat.h:5, from arch/powerpc/kernel/irq.c:35:
./include/linux/dma-mapping.h: In function =E2=80=98dma_map_resource=E2=
=80=99:
Quoted 1 line
./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned e=
xpression >=3D 0 is always true [-Werror=3Dtype-limits]
arch/powerpc/include/asm/p=
Quoted 9 lines
#define pfn_valid(pfn) ((pfn) >=3D ARCH_PFN_OFFSET && (pfn) < max_mapnr) ^Suggested-by Segher Boessenkool <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/include/asm/page.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/powerpc/include/asm/p= renamed from arch/powerpc/include/asm/page.h +0
-0
age.h
arch/powerpc/include/asm/page.h extern long long virt_phys_offset; · line 130
Quoted 10 lines
index
8da5d4c1cab2..19dea64e7ed2100644arch/powerpc/include/asm/page.h +0
-0@@ -126,7 +126,8 @@ extern long long virt_phys_offset;=20=20
#ifdefCONFIG_FLATMEM
#defineARCH_PFN_OFFSET((unsigned long)(MEMORY_START >>PAGE_SHIFT))arch/powerpc/include/asm/page.h +2
-1-#define pfn_valid (pfn) ((pfn) >=3D ARCH_PFN_OFFSET && (pfn) < max_mapnr) +#define pfn_valid (pfn) \ + (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
I'm not a big fan of this one, because the original code is *far* more obvious as to what it's doing.
I'm not sure if we can make this one a static inline, or whether that would help, but it would be worth investigating.
cheers
Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
Le 04/03/2018 à 11:55, Michael Ellerman a écrit :
Quoted 37 lines
Quoted 25 lines · Mathieu Malaterre
Rewrite comparison since all values compared are of type
unsigned long.Fix a warning (treated as error in W=1):
CC
arch/powerpc/kernel/irq.o
In file included from./include/linux/bug.h:5:0,from ./include/linux/cpumask.h:13, from ./include/linux/smp.h:13, from ./include/linux/kernel_stat.h:5, from arch/powerpc/kernel/irq.c:35: ./include/linux/dma-mapping.h: In function ‘dma_map_resource’: ./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits] #define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr) ^Suggested-by Segher Boessenkool <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/include/asm/page.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/powerpc/include/asm/page.h +0
-0arch/powerpc/include/asm/page.h extern long long virt_phys_offset; · line 130
Quoted 6 lines
arch/powerpc/include/asm/page.h +2
-1@@ -126,7 +126,8 @@ extern long long virt_phys_offset;#ifdef CONFIG_FLATMEM #define ARCH_PFN_OFFSET ((unsigned long)(MEMORY_START >> PAGE_SHIFT)) -#define pfn_valid (pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr) +#define pfn_valid (pfn) \ + (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))I'm not a big fan of this one, because the original code is *far* more obvious as to what it's doing.
Quoted 3 lines
I'm not sure if we can make this one a static inline, or whether that would help, but it would be worth investigating.
The following seems to give a good result:
arch/powerpc/include/asm/page.h +9 -1
@@ -126,7 +126,15 @@ extern long long virt_phys_offset;#ifdef CONFIG_FLATMEM #define ARCH_PFN_OFFSET ((unsigned long)(MEMORY_START >> PAGE_SHIFT)) -#define pfn_valid (pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr) +#ifndef __ASSEMBLY__ +extern unsigned long max_mapnr; +static inline bool pfn_valid (unsigned long pfn) +{ +unsigned long min_pfn =ARCH_PFN_OFFSET ; + +return pfn >= min_pfn && pfn < max_mapnr; +} +#endif #endif #define virt_to_pfn (kaddr) (__pa(kaddr) >> PAGE_SHIFT)
Christophe
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
PATCH v2 5/21 powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
In reply to Mathieu MalaterreRewrite comparison since all values compared are of type unsigned long.
Instead of using unsigned properties and rewriting the original code as:
(originally suggested by Segher Boessenkool <[email protected]>)
#define pfn_valid(pfn) \
(((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))Prefer a static inline function to make code as readable as possible.
Fix a warning (treated as error in W=1):
CC arch/powerpc/kernel/irq.o
In file included from ./include/linux/bug.h:5:0,
from ./include/linux/cpumask.h:13,
from ./include/linux/smp.h:13,
from ./include/linux/kernel_stat.h:5,
from arch/powerpc/kernel/irq.c:35:
./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
#define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
^--- arch/powerpc/include/asm/page.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
arch/powerpc/include/asm/page.h +9 -1
@@ -126,7 +126,15 @@ extern long long virt_phys_offset;#ifdef CONFIG_FLATMEM #define ARCH_PFN_OFFSET ((unsigned long)(MEMORY_START >> PAGE_SHIFT)) -#define pfn_valid (pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr) +#ifndef __ASSEMBLY__ +extern unsigned long max_mapnr; +static inline bool pfn_valid (unsigned long pfn) +{ +unsigned long min_pfn =ARCH_PFN_OFFSET ; + +return pfn >= min_pfn && pfn < max_mapnr; +} +#endif #endif #define virt_to_pfn (kaddr) (__pa(kaddr) >> PAGE_SHIFT)
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [v2, 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
Quoted 22 lines · Mathieu Malaterre
Rewrite comparison since all values compared are of type
unsigned long.Instead of using unsigned properties and rewriting the original code as:
(originally suggested by Segher Boessenkool <[email protected]>)#define pfn_valid(pfn) \ (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))Prefer a static inline function to make code as readable as possible.
Fix a warning (treated as error in W=1):
CC
arch/powerpc/kernel/irq.o
In file included from./include/linux/bug.h:5:0,from ./include/linux/cpumask.h:13, from ./include/linux/smp.h:13, from ./include/linux/kernel_stat.h:5, from arch/powerpc/kernel/irq.c:35: ./include/linux/dma-mapping.h: In function ‘dma_map_resource’: ./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits] #define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr) ^
Quoted 2 lines
Suggested-by Christophe Leroy <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/603b892200e653dd7e86a0e4a31556
cheers
PATCH 6/21 powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
In reply to Mathieu MalaterreRewrite check size - 1 <= Y as size < Y since size is unsigned value. Fix warning (treated as error in W=1):
CC arch/powerpc/kernel/signal_32.o
In file included from ./include/linux/uaccess.h:14:0,
from ./include/asm-generic/termios-base.h:8,
from ./arch/powerpc/include/asm/termios.h:20,
from ./include/uapi/linux/termios.h:6,
from ./include/linux/tty.h:7,
from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:
./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
(((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
^./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
__access_ok((__force unsigned long)(addr), (size), get_fs())) ^~~~~~~~~~~./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’
if (access_ok(VERIFY_READ, __gu_addr, (size))) \
^~~~~~~~~./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
__get_user_check((x), (ptr), sizeof(*(ptr)))
^~~~~~~~~~~~~~~~./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
if (get_user(termios->c_line, &termio->c_line) < 0)
^~~~~~~~
[...]
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/uaccess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/powerpc/include/asm/uaccess.h +1 -1
@@ -49,7 +49,7 @@#define __access_ok (addr, size, segment) \ (((addr) <= (segment).seg) && \ - (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) + (((size) == 0) || ((size) < ((segment).seg - (addr)))))#endif
Signature · 1 line
2.11.0
Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
Quoted 39 lines
Rewrite check size - 1 <= Y as size < Y since
sizeis unsigned value. Fix warning (treated as error in W=1):CC
arch/powerpc/kernel/signal_32.o
In file included from./include/linux/uaccess.h:14:0,from ./include/asm-generic/termios-base.h:8, from ./arch/powerpc/include/asm/termios.h:20, from ./include/uapi/linux/termios.h:6, from ./include/linux/tty.h:7, from arch/powerpc/kernel/signal_32.c:36: ./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’: ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits] (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) ^ ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’ __access_ok((__force unsigned long)(addr), (size), get_fs())) ^~~~~~~~~~~ ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’ if (access_ok(VERIFY_READ, __gu_addr, (size))) \ ^~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
__get_user_check((x), (ptr), sizeof(*(ptr)))
^~~~~~~~~~~~~~~~./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
if (get_user(termios->c_line, &termio->c_line)< 0)^~~~~~~~[...]
cc1: all warnings being treated as errorsSigned-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/include/asm/uaccess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/powerpc/include/asm/uaccess.h +0
-0
arch/powerpc/include/asm/uaccess.h line 52
Quoted 5 lines
arch/powerpc/include/asm/uaccess.h +1
-1@@ -49,7 +49,7 @@#define __access_ok (addr, size, segment) \ (((addr) <= (segment).seg) && \ - (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) + (((size) == 0) || ((size) < ((segment).seg - (addr)))))
IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????
Christophe
Quoted 4 lines
arch/powerpc/include/asm/uaccess.h +0
-0#endif
Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
Le 26/02/2018 à 07:34, Christophe LEROY a écrit :
Quoted 37 lines
Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
Quoted 34 lines
Rewrite check size - 1 <= Y as size < Y since
sizeis unsigned value. Fix warning (treated as error in W=1):CC
arch/powerpc/kernel/signal_32.o
In file included from./include/linux/uaccess.h:14:0,
from./include/asm-generic/termios-base.h:8,
from./arch/powerpc/include/asm/termios.h:20,
from./include/uapi/linux/termios.h:6,
from./include/linux/tty.h:7,
fromarch/powerpc/kernel/signal_32.c:36:./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits] (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
^./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
__access_ok((__force unsigned long)(addr), (size), get_fs()))
^~~~~~~~~~~./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’
if (access_ok(VERIFY_READ, __gu_addr, (size))) \
^~~~~~~~~./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’__get_user_check((x), (ptr), sizeof(*(ptr)))
^~~~~~~~~~~~~~~~./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
if (get_user(termios->c_line, &termio->c_line)< 0)
^~~~~~~~
[...]
cc1: all warnings being treated as errorsSigned-off-by Mathieu Malaterre <[email protected]>---
arch/powerpc/include/asm/uaccess.h| 2 +-1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 11 lines
arch/powerpc/include/asm/uaccess.h line 51
Quoted 9 lines
arch/powerpc/include/asm/uaccess.h +1
-1@@ -49,7 +49,7 @@ #define __access_ok(addr, size, segment) \ (((addr) <= (segment).seg) && \ - (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) + (((size) == 0) || ((size) < ((segment).seg - (addr)))))IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????
Note that I already try to submit a fix for this warning 3 years ago (https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the following comment:
Again, I don't think Linux enables this warning. What did you do to produce this? In any case, it's a bad warning that doesn't take macros into account, and the answer is not to make the code less clear by hiding the fact that zero is a special case.
Christophe
Quoted 5 lines
Christophe
Quoted 2 lines
arch/powerpc/include/asm/uaccess.h +0
-0#endif
Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
Quoted 9 lines · Christophe LEROY
Le 26/02/2018 =C3=A0 07:34, Christophe LEROY a =C3=A9crit :
Quoted 6 lines
Le 25/02/2018 =C3=A0 18:22, Mathieu Malaterre a =C3=A9crit :
Quoted 2 lines
Rewrite check size - 1 <=3D Y as size < Y since
sizeis unsigned valu=
e.
Quoted 12 lines
Quoted 12 lines
Quoted 4 lines
Fix warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/signal_32.o
In file included from./include/linux/uaccess.h:14:0,Quoted 8 lines
from ./include/asm-generic/termios-base.h:8, from ./arch/powerpc/include/asm/termios.h:20, from ./include/uapi/linux/termios.h:6, from ./include/linux/tty.h:7, from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function =E2=80=98user_termio_to_kernel_termios=E2=80=99:./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsign=
ed
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
expression >=3D 0 is always true [-Werror=3Dtype-limits]
(((size) =3D=3D 0) || (((size) - 1) <=3D ((segment).seg - (addr))))=
)
Quoted 25 lines
Quoted 25 lines
Quoted 25 lines
^ ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro =E2=80=98__access_ok=E2=80=99 __access_ok((__force unsigned long)(addr), (size), get_fs())) ^~~~~~~~~~~ ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro =E2=80=98access_ok=E2=80=99 if (access_ok(VERIFY_READ, __gu_addr, (size))) \ ^~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro =E2=80=98__get_user_check=E2=80=99
__get_user_check((x), (ptr), sizeof(*(ptr)))^~~~~~~~~~~~~~~~./include/asm-generic/termios-base.h:36:6: note: in expansion of macro =E2=80=98get_user=E2=80=99
if (get_user(termios->c_line, &termio->c_line)< 0)^~~~~~~~[...]
cc1: all warnings being treated as errorsSigned-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/include/asm/uaccess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 9 lines
Quoted 9 lines
arch/powerpc/include/asm/uaccess.h line 51
Quoted 9 lines
arch/powerpc/include/asm/uaccess.h +0
-1@@ -49,7 +49,7 @@#define __access_ok (addr, size, segment) \ (((addr) <=3D (segment).seg) && \ - (((size) =3D=3D 0) || (((size) - 1) <=3D ((segment).seg - (addr))=
)))
Quoted 5 lines
Quoted 4 lines
arch/powerpc/include/asm/uaccess.h line 51
Quoted 1 line
arch/powerpc/include/asm/uaccess.h +1
-0+ (((size) =3D=3D 0) || ((size) < ((segment).seg - (addr)))))IIUC, ((2 - 1) <=3D 1) is the same as (2 < 1) ?????
The whole series was pretty mediocre, but this one was actually pretty destructive. Thanks for catching this.
Quoted 9 lines
Note that I already try to submit a fix for this warning 3 years ago
(https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the following comment:Again, I don't think Linux enables this warning. What did you do to produce this? In any case, it's a bad warning that doesn't take macros into account, and the answer is not to make the code less clear by hiding the fact that zero is a special case.
Right. I'll try to see how to make W=3D1 run without error with an alternate solution.
Quoted 9 lines
Christophe
Quoted 5 lines
Christophe
Quoted 2 lines
arch/powerpc/include/asm/uaccess.h +0
-0#endif
Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
On Mon, Feb 26, 2018 at 8:44 AM, Mathieu Malaterre <[email protected]> wrote= :
Quoted 11 lines
Quoted 9 lines · Christophe LEROY
Le 26/02/2018 =C3=A0 07:34, Christophe LEROY a =C3=A9crit :
Quoted 6 lines
Le 25/02/2018 =C3=A0 18:22, Mathieu Malaterre a =C3=A9crit :
Quoted 2 lines
Rewrite check size - 1 <=3D Y as size < Y since
sizeis unsigned val=
ue.
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 4 lines
Fix warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/signal_32.o
In file included from./include/linux/uaccess.h:14:0,Quoted 8 lines
from ./include/asm-generic/termios-base.h:8, from ./arch/powerpc/include/asm/termios.h:20, from ./include/uapi/linux/termios.h:6, from ./include/linux/tty.h:7, from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function =E2=80=98user_termio_to_kernel_termios=E2=80=99:./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsig=
ned
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
expression >=3D 0 is always true [-Werror=3Dtype-limits]
(((size) =3D=3D 0) || (((size) - 1) <=3D ((segment).seg - (addr)))=
))
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
^ ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro =E2=80=98__access_ok=E2=80=99 __access_ok((__force unsigned long)(addr), (size), get_fs())) ^~~~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macr=
o
Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
=E2=80=98access_ok=E2=80=99
if (access_ok(VERIFY_READ, __gu_addr, (size))) \ ^~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro =E2=80=98__get_user_check=E2=80=99
__get_user_check((x), (ptr), sizeof(*(ptr)))^~~~~~~~~~~~~~~~./include/asm-generic/termios-base.h:36:6: note: in expansion of macro =E2=80=98get_user=E2=80=99
if (get_user(termios->c_line, &termio->c_line)< 0)^~~~~~~~[...]
cc1: all warnings being treated as errorsSigned-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/include/asm/uaccess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
arch/powerpc/include/asm/uaccess.h line 51
Quoted 9 lines
arch/powerpc/include/asm/uaccess.h +0
-1@@ -49,7 +49,7 @@#define __access_ok (addr, size, segment) \ (((addr) <=3D (segment).seg) && \ - (((size) =3D=3D 0) || (((size) - 1) <=3D ((segment).seg - (addr)=
))))
Quoted 12 lines
Quoted 5 lines
Quoted 4 lines
arch/powerpc/include/asm/uaccess.h line 51
Quoted 1 line
arch/powerpc/include/asm/uaccess.h +1
-0+ (((size) =3D=3D 0) || ((size) < ((segment).seg - (addr)))))IIUC, ((2 - 1) <=3D 1) is the same as (2 < 1) ?????
The whole series was pretty mediocre, but this one was actually pretty destructive. Thanks for catching this.
Quoted 3 lines
Note that I already try to submit a fix for this warning 3 years ago
(https://patchwork.ozlabs.org/patch/418075/) and it was rejected with th=
e
Quoted 1 line
Quoted 1 line
following comment:
Tested again today with gcc 6.3.0 and gcc is still producing the original warning (treated as error).
Quoted 3 lines
Quoted 3 lines
Again, I don't think Linux enables this warning. What did you do to produce this? In any case, it's a bad warning that doesn't take macros into account, and the answer is not to make the code less clear by hidin=
g
Quoted 4 lines
Quoted 1 line
the fact that zero is a special case.
Right. I'll try to see how to make W=3D1 run without error with an alternate solution.
So the other alternative is to update a bunch of ppc32 defconfig(s)
with: CONFIG_PPC_DISABLE_WERROR=3Dy.
Would that be preferable ?
Quoted 9 lines
Quoted 9 lines
Christophe
Quoted 5 lines
Christophe
Quoted 2 lines
arch/powerpc/include/asm/uaccess.h +0
-0#endif
Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
Le 26/02/2018 à 18:50, Mathieu Malaterre a écrit :
Quoted 70 lines · Mathieu Malaterre
Quoted 69 lines
Quoted 48 lines · Christophe LEROY
Quoted 45 lines · Christophe LEROY
Quoted 41 lines · Mathieu Malaterre
Rewrite check size - 1 <= Y as size < Y since
sizeis unsigned value. Fix warning (treated as error in W=1):CC
arch/powerpc/kernel/signal_32.o
In file included from./include/linux/uaccess.h:14:0,from ./include/asm-generic/termios-base.h:8, from ./arch/powerpc/include/asm/termios.h:20, from ./include/uapi/linux/termios.h:6, from ./include/linux/tty.h:7, from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits](((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) ^ ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’ __access_ok((__force unsigned long)(addr), (size), get_fs())) ^~~~~~~~~~~ ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’ if (access_ok(VERIFY_READ, __gu_addr, (size))) \ ^~~~~~~~~ ./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’ __get_user_check((x), (ptr), sizeof(*(ptr))) ^~~~~~~~~~~~~~~~ ./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’ if (get_user(termios->c_line, &termio->c_line) < 0) ^~~~~~~~[...]
cc1: all warnings being treated as errorsSigned-off-by Mathieu Malaterre <[email protected]>--- arch/powerpc/include/asm/uaccess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 14 lines
Quoted 5 lines
Quoted 5 lines
arch/powerpc/include/asm/uaccess.h +0
-0Quoted 8 lines
arch/powerpc/include/asm/uaccess.h line 51
Quoted 5 lines
arch/powerpc/include/asm/uaccess.h +1
-1@@ -49,7 +49,7 @@#define __access_ok (addr, size, segment) \ (((addr) <= (segment).seg) && \ - (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) + (((size) == 0) || ((size) < ((segment).seg - (addr)))))IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????
The whole series was pretty mediocre, but this one was actually pretty destructive. Thanks for catching this.
Quoted 1 line
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Note that I already try to submit a fix for this warning 3 years ago
(https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the following comment:Tested again today with gcc 6.3.0 and gcc is still producing the original warning (treated as error).
That's right, it seems that recent versions of gcc are not happy anymore with that change.
Maybe Segher has a suggestion for that one ?
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Again, I don't think Linux enables this warning. What did you do to produce this? In any case, it's a bad warning that doesn't take macros into account, and the answer is not to make the code less clear by hiding the fact that zero is a special case.
Quoted 7 lines
Quoted 2 lines
Right. I'll try to see how to make W=1 run without error with an alternate solution.
So the other alternative is to update a bunch of ppc32 defconfig(s)
with:CONFIG_PPC_DISABLE_WERROR=y.Would that be preferable ?
No I don't think it is the solution. PPC is built with WERROR in order to catch warnings on real problems.
You could disable it selectively when you want to run 'make W=1' and see all possible warnings, then select by yourself which warnings are worth fixing up.
Christophe
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Christophe
Quoted 5 lines
Christophe
Quoted 2 lines
arch/powerpc/include/asm/uaccess.h +0
-0#endif
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
Quoted 9 lines · christophe leroy
Le 26/02/2018 à 18:50, Mathieu Malaterre a écrit :
Quoted 8 lines
Quoted 5 lines · Mathieu Malaterre
Quoted 3 lines · Christophe LEROY
Note that I already try to submit a fix for this warning 3 years ago
(https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the following comment:
Quoted 7 lines
Quoted 2 lines
Tested again today with gcc 6.3.0 and gcc is still producing the original warning (treated as error).
That's right, it seems that recent versions of gcc are not happy anymore with that change.
Maybe Segher has a suggestion for that one ?
Your patch:
#define __access_ok(addr, size, segment) \
(((addr) <= (segment).seg) && \
- (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) + (((size) <= 1) || (((size) - 1) <= ((segment).seg - (addr)))))
Is there any reason to write this as a macro? Let's make this more readable:
static inline int __access_ok(unsigned long addr, unsigned long size,
mm_segment_t seg)
{
if (addr > seg.seg)
return 0;
return (size == 0 || size - 1 <= seg.seg - addr);
}and I think we are done already, or will this warn for any input?
Segher
PATCH v2 6/21 powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
In reply to Mathieu MalaterreRewrite function-like macro into regular static inline function to avoid a warning during macro expansion.
Fix warning (treated as error in W=1):
CC arch/powerpc/kernel/signal_32.o
In file included from ./include/linux/uaccess.h:14:0,
from ./include/asm-generic/termios-base.h:8,
from ./arch/powerpc/include/asm/termios.h:20,
from ./include/uapi/linux/termios.h:6,
from ./include/linux/tty.h:7,
from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:
./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
(((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
^./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
__access_ok((__force unsigned long)(addr), (size), get_fs())) ^~~~~~~~~~~./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’
if (access_ok(VERIFY_READ, __gu_addr, (size))) \
^~~~~~~~~./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
__get_user_check((x), (ptr), sizeof(*(ptr)))
^~~~~~~~~~~~~~~~./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
if (get_user(termios->c_line, &termio->c_line) < 0)
^~~~~~~~
[...]
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/uaccess.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
arch/powerpc/include/asm/uaccess.h +7 -3
@@ -47,9 +47,13 @@#else -#define __access_ok (addr, size, segment) \ - (((addr) <= (segment).seg) && \ - (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) +static inline int __access_ok (unsigned long addr,unsigned long size, +mm_segment_t seg) +{ +if (addr > seg.seg) +return 0 ; +return (size ==0 || size -1 <= seg.seg - addr); +}#endif
Signature · 1 line
2.11.0
Re: [PATCH v2 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
Le 02/03/2018 à 20:50, Mathieu Malaterre a écrit :
Quoted 26 lines
Rewrite function-like macro into regular static inline function to avoid a warning during macro expansion.
Fix warning (treated as error in W=1):CC
arch/powerpc/kernel/signal_32.o
In file included from./include/linux/uaccess.h:14:0,from ./include/asm-generic/termios-base.h:8, from ./arch/powerpc/include/asm/termios.h:20, from ./include/uapi/linux/termios.h:6, from ./include/linux/tty.h:7, from arch/powerpc/kernel/signal_32.c:36: ./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’: ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits] (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) ^ ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’ __access_ok((__force unsigned long)(addr), (size), get_fs())) ^~~~~~~~~~~ ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’ if (access_ok(VERIFY_READ, __gu_addr, (size))) \ ^~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
__get_user_check((x), (ptr), sizeof(*(ptr)))
^~~~~~~~~~~~~~~~./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
if (get_user(termios->c_line, &termio->c_line)< 0)
Quoted 6 lines
^~~~~~~~[...]
cc1: all warnings being treated as errorsSuggested-by Segher Boessenkool <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>
Quoted 16 lines
--- arch/powerpc/include/asm/uaccess.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)arch/powerpc/include/asm/uaccess.h +0
-3@@ -47,9 +47,13 @@#else -#define __access_ok (addr, size, segment) \ - (((addr) <= (segment).seg) && \ - (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
arch/powerpc/include/asm/uaccess.h line 59
Quoted 10 lines
arch/powerpc/include/asm/uaccess.h +7
-0+static inline int __access_ok (unsigned long addr,unsigned long size, +mm_segment_t seg) +{ +if (addr > seg.seg) +return 0 ; +return (size ==0 || size -1 <= seg.seg - addr); +}#endif
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
Michael Ellerman
AppliedRe: [v2, 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
In reply to Mathieu MalaterreQuoted 26 lines · Mathieu Malaterre
Rewrite function-like macro into regular static inline function to avoid a warning during macro expansion.
Fix warning (treated as error in W=1):CC
arch/powerpc/kernel/signal_32.o
In file included from./include/linux/uaccess.h:14:0,from ./include/asm-generic/termios-base.h:8, from ./arch/powerpc/include/asm/termios.h:20, from ./include/uapi/linux/termios.h:6, from ./include/linux/tty.h:7, from arch/powerpc/kernel/signal_32.c:36: ./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’: ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits] (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr))))) ^
./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
__access_ok((__force unsigned long)(addr), (size), get_fs())) ^~~~~~~~~~~./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’if (access_ok(VERIFY_READ, __gu_addr, (size))) \ ^~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
__get_user_check((x), (ptr), sizeof(*(ptr)))
^~~~~~~~~~~~~~~~./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
if (get_user(termios->c_line, &termio->c_line)< 0)
Quoted 7 lines
^~~~~~~~[...]
cc1: all warnings being treated as errorsSuggested-by Segher Boessenkool <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Reviewed-by Christophe Leroy <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/ef85dffd4251ff6c23056651f6f83b
cheers
PATCH 7/21 powerpc: Make functions flipper_pic_init & ug_udbg_putc static
In reply to Mathieu MalaterreChange signature of two functions, adding static keyword to prevent the following two warnings (treated as errors on W=1):
CC kernel/sys.oarch/powerpc/platforms/embedded6xx/flipper-pic.c:135:28: error: no previous prototype for ‘flipper_pic_init’ [-Werror=missing-prototypes]
struct irq_domain * __init flipper_pic_init(struct device_node *np)
^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
and
CC arch/powerpc/platforms/embedded6xx/usbgecko_udbg.oarch/powerpc/platforms/embedded6xx/usbgecko_udbg.c:172:6: error: no previous prototype for ‘ug_udbg_putc’ [-Werror=missing-prototypes]
void ug_udbg_putc(char ch)
^~~~~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/platforms/embedded6xx/flipper-pic.c | 2 +- arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
arch/powerpc/platforms/embedded6xx/flipper-pic.c +1 -1
@@ -132,7 +132,7 @@ static void __flipper_quiesce(void __iomem *io_base)out_be32 (io_base +FLIPPER_ICR ,0xffffffff ); } -struct irq_domain *__init flipper_pic_init (struct device_node *np) +static struct irq_domain *__init flipper_pic_init (struct device_node *np) {struct device_node *pi;struct irq_domain *irq_domain =NULL ;
arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c +1 -1
@@ -169,7 +169,7 @@ static int ug_getc(void)/* * Transmits a character. */ -void ug_udbg_putc (char ch) +static void ug_udbg_putc (char ch) {ug_putc (ch); }
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [07/21] powerpc: Make functions flipper_pic_init & ug_udbg_putc static
Quoted 11 lines · Mathieu Malaterre
Change signature of two functions, adding static keyword to prevent the following two warnings (treated as errors on W=1):
CC
kernel/sys.oarch/powerpc/platforms/embedded6xx/flipper-pic.c:135:28: error: no previous prototype for ‘flipper_pic_init’ [-Werror=missing-prototypes]
structirq_domain*__initflipper_pic_init(struct device_node *np)^~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
and
Quoted 7 lines
CC
arch/powerpc/platforms/embedded6xx/usbgecko_udbg.oarch/powerpc/platforms/embedded6xx/usbgecko_udbg.c:172:6: error: no previous prototype for ‘ug_udbg_putc’ [-Werror=missing-prototypes]
voidug_udbg_putc(char ch)^~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8b51e679a54e808bdf1f2cc6552cf2
cheers
PATCH 8/21 powerpc: Make function __giveup_fpu static
In reply to Mathieu MalaterreWhen CONFIG_PPC_FPU is defined the prototype exposed in asm/switch_to.h is:
extern void giveup_fpu(struct task_struct *);
Change the function prototype of __giveup_fpu to static. Fix warning (treated as error in W=1):
CC arch/powerpc/kernel/process.oarch/powerpc/kernel/process.c:176:6: error: no previous prototype for ‘__giveup_fpu’ [-Werror=missing-prototypes]
void __giveup_fpu(struct task_struct *tsk)
^~~~~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/kernel/process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/powerpc/kernel/process.c +1 -1
@@ -173,7 +173,7 @@ void __msr_check_and_clear(unsigned long bits)EXPORT_SYMBOL (__msr_check_and_clear);#ifdef CONFIG_PPC_FPU -void __giveup_fpu (struct task_struct *tsk) +static void __giveup_fpu (struct task_struct *tsk) {unsigned long msr;
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [08/21] powerpc: Make function __giveup_fpu static
Quoted 7 lines · Mathieu Malaterre
When
CONFIG_PPC_FPUis defined the prototype exposed inasm/switch_to.his:extern void giveup_fpu(struct task_struct *);Change the function prototype of
__giveup_fputo static. Fix warning (treated as error in W=1):
Quoted 7 lines
CC
arch/powerpc/kernel/process.oarch/powerpc/kernel/process.c:176:6: error: no previous prototype for ‘__giveup_fpu’ [-Werror=missing-prototypes]
void__giveup_fpu(struct task_struct *tsk)^~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/1cdf039bf82a39d816ca4b8161b01c
cheers
PATCH 9/21 powerpc: Make function save_all static
In reply to Mathieu MalaterreSince function save_all is not meant to be exported, change the signature to static. Fix warning (treated as error with W=1):
CC arch/powerpc/kernel/process.oarch/powerpc/kernel/process.c:559:6: error: no previous prototype for ‘save_all’ [-Werror=missing-prototypes]
void save_all(struct task_struct *tsk)
^~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/kernel/process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/powerpc/kernel/process.c +1 -1
@@ -556,7 +556,7 @@ void restore_math(struct pt_regs *regs) regs->msr = msr; } -void save_all (struct task_struct *tsk) +static void save_all (struct task_struct *tsk) {unsigned long usermsr;
Signature · 1 line
2.11.0
PATCH 10/21 powerpc: Add missing prototype for slb_miss_bad_addr
In reply to Mathieu MalaterreIn commit f0f558b131db ("powerpc/mm: Preserve CFAR value on SLB miss caused by access to bogus address"), the function slb_miss_bad_addr was added without a prototype. This commit adds it.
Fix a warning (treated as error in W=1):
CC arch/powerpc/kernel/traps.oarch/powerpc/kernel/traps.c:1498:6: error: no previous prototype for ‘slb_miss_bad_addr’ [-Werror=missing-prototypes]
void slb_miss_bad_addr(struct pt_regs *regs)
^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/asm-prototypes.h | 1 + 1 file changed, 1 insertion(+)
arch/powerpc/include/asm/asm-prototypes.h +1 -0
@@ -62,6 +62,7 @@ void RunModeException(struct pt_regs *regs);void single_step_exception (struct pt_regs *regs);void program_check_exception (struct pt_regs *regs);void alignment_exception (struct pt_regs *regs); +void slb_miss_bad_addr (struct pt_regs *regs);void StackOverflow (struct pt_regs *regs);void nonrecoverable_exception (struct pt_regs *regs);void kernel_fp_unavailable_exception (struct pt_regs *regs);
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [10/21] powerpc: Add missing prototype for slb_miss_bad_addr
Quoted 6 lines · Mathieu Malaterre
In commit
f0f558b131db("powerpc/mm: Preserve CFAR value on SLB miss caused by access to bogus address"), the functionslb_miss_bad_addrwas added without a prototype. This commit adds it.Fix a warning (treated as error in W=1):
Quoted 7 lines
CC
arch/powerpc/kernel/traps.oarch/powerpc/kernel/traps.c:1498:6: error: no previous prototype for ‘slb_miss_bad_addr’ [-Werror=missing-prototypes]
voidslb_miss_bad_addr(struct pt_regs *regs)^~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/45b4d27a3897d6094bcf84bc877439
cheers
PATCH 11/21 powerpc: Add missing prototype for hdec_interrupt
In reply to Mathieu MalaterreIn commit dabe859ec636 ("powerpc: Give hypervisor decrementer interrupts their own handler") an empty body function was added, but no prototype was declared. Fix warning (treated as error in W=1):
CC arch/powerpc/kernel/time.oarch/powerpc/kernel/time.c:629:6: error: no previous prototype for ‘hdec_interrupt’ [-Werror=missing-prototypes]
void hdec_interrupt(struct pt_regs *regs)
^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/time.h | 1 + 1 file changed, 1 insertion(+)
arch/powerpc/include/asm/time.h +1 -0
@@ -31,6 +31,7 @@ extern void to_tm(int tim, struct rtc_time * tm);extern void tick_broadcast_ipi_handler (void );extern void generic_calibrate_decr (void ); +extern void hdec_interrupt (struct pt_regs *regs);/* Some sane defaults: 125 MHz timebase, 1GHz processor */ extern unsigned long ppc_proc_freq;
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [11/21] powerpc: Add missing prototype for hdec_interrupt
Quoted 11 lines · Mathieu Malaterre
In commit
dabe859ec636("powerpc: Give hypervisor decrementer interrupts their own handler") an empty body function was added, but no prototype was declared. Fix warning (treated as error in W=1):CC
arch/powerpc/kernel/time.oarch/powerpc/kernel/time.c:629:6: error: no previous prototype for ‘hdec_interrupt’ [-Werror=missing-prototypes]
voidhdec_interrupt(struct pt_regs *regs)^~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8b604faff7d421904ebd1fa65d642f
cheers
PATCH 12/21 powerpc: Add missing prototype for time_init
In reply to Mathieu MalaterreThe function time_init did not have a prototype defined in the time.h header. Fix the following warning (treated as error in W=1):
CC arch/powerpc/kernel/time.oarch/powerpc/kernel/time.c:1068:13: error: no previous prototype for ‘time_init’ [-Werror=missing-prototypes]
void __init time_init(void)
^~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/time.h | 1 + 1 file changed, 1 insertion(+)
arch/powerpc/include/asm/time.h +1 -0
@@ -205,6 +205,7 @@ struct cpu_usage { DECLARE_PER_CPU(struct cpu_usage, cpu_usage_array);extern void secondary_cpu_time_init (void ); +extern void __init time_init (void );DECLARE_PER_CPU (u64, decrementers_next_tb);
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [12/21] powerpc: Add missing prototype for time_init
Quoted 10 lines · Mathieu Malaterre
The function
time_initdid not have a prototype defined in thetime.hheader. Fix the following warning (treated as error in W=1):CC
arch/powerpc/kernel/time.oarch/powerpc/kernel/time.c:1068:13: error: no previous prototype for ‘time_init’ [-Werror=missing-prototypes]
void__inittime_init(void)^~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/848092faa0c7687a99bf465808f7da
cheers
PATCH 13/21 powerpc: Add missing prototype for arch_dup_task_struct
In reply to Mathieu MalaterreIn commit 55ccf3fe3f9a ("fork: move the real prepare_to_copy() users to arch_dup_task_struct()") a new arch_dup_task_struct was added without a prototype declared in thread_info.h header. Fix the following warning (treated as error in W=1):
CC arch/powerpc/kernel/process.oarch/powerpc/kernel/process.c:1609:5: error: no previous prototype for ‘arch_dup_task_struct’ [-Werror=missing-prototypes]
int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/thread_info.h | 1 + 1 file changed, 1 insertion(+)
arch/powerpc/include/asm/thread_info.h +1 -0
@@ -70,6 +70,7 @@ static inline struct thread_info *current_thread_info(void) return (struct thread_info *)val; } +extern int arch_dup_task_struct (struct task_struct *dst,struct task_struct *src);#endif /* __ASSEMBLY__ */ /*
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [13/21] powerpc: Add missing prototype for arch_dup_task_struct
Quoted 5 lines · Mathieu Malaterre
In commit
55ccf3fe3f9a("fork: move the realprepare_to_copy()users toarch_dup_task_struct()") a newarch_dup_task_structwas added without a prototype declared inthread_info.hheader. Fix the following warning (treated as error in W=1):
Quoted 7 lines
CC
arch/powerpc/kernel/process.oarch/powerpc/kernel/process.c:1609:5: error: no previous prototype for ‘arch_dup_task_struct’ [-Werror=missing-prototypes]
intarch_dup_task_struct(struct task_struct *dst, struct task_struct *src)^~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/fd70d9f96d8182a67e28b99c999157
cheers
PATCH 14/21 powerpc: Add missing prototype for arch_irq_work_raise
In reply to Mathieu MalaterreIn commit 4f8b50bbbe63 ("irq_work, ppc: Fix up arch hooks") a new function arch_irq_work_raise was added without a prototype in header irq_work.h.
Fix the following warning (treated as error in W=1):
CC arch/powerpc/kernel/time.oarch/powerpc/kernel/time.c:523:6: error: no previous prototype for ‘arch_irq_work_raise’ [-Werror=missing-prototypes]
void arch_irq_work_raise(void)
^~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/irq_work.h | 1 + 1 file changed, 1 insertion(+)
arch/powerpc/include/asm/irq_work.h +1 -0
@@ -6,5 +6,6 @@ static inline bool arch_irq_work_has_interrupt(void) {return true; } +extern void arch_irq_work_raise (void );#endif /* _ASM_POWERPC_IRQ_WORK_H */
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [14/21] powerpc: Add missing prototype for arch_irq_work_raise
Quoted 5 lines · Mathieu Malaterre
In commit
4f8b50bbbe63("irq_work, ppc: Fix up arch hooks") a new functionarch_irq_work_raisewas added without a prototype in headerirq_work.h.Fix the following warning (treated as error in W=1):
Quoted 7 lines
CC
arch/powerpc/kernel/time.oarch/powerpc/kernel/time.c:523:6: error: no previous prototype for ‘arch_irq_work_raise’ [-Werror=missing-prototypes]
voidarch_irq_work_raise(void)^~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/f5246862f82f1e16bbf84cda4cddf2
cheers
PATCH 15/21 powerpc: Add missing prototype for MMU_setup
In reply to Mathieu MalaterreAdd a function declaration for MMU_setup at the beginning of the file to fix a warning (treated as error in W=1):
CC kernel/sys.oarch/powerpc/mm/init_32.c:102:13: error: no previous prototype for ‘MMU_setup’ [-Werror=missing-prototypes]
void __init MMU_setup(void)
^~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/mm/init_32.c | 1 + 1 file changed, 1 insertion(+)
arch/powerpc/mm/init_32.c +1 -0
@@ -79,6 +79,7 @@ EXPORT_SYMBOL(agp_special_page);#endif void MMU_init (void ); +void __init MMU_setup (void );/* * this tells the system to map all of ram with the segregs
Signature · 1 line
2.11.0
Michael Ellerman
reviewerRe: [PATCH 15/21] powerpc: Add missing prototype for MMU_setup
Quoted 5 lines · Mathieu Malaterre
Add a function declaration for
MMU_setupat the beginning of the file to fix a warning (treated as error in W=3D1):CC
kernel/sys.oarch/powerpc/mm/init_32.c:102:13: error: no previous prototype for =E2=80=
=98MMU_setup=E2=80=99 [-Werror=3Dmissing-prototypes]
Quoted 3 lines
void
__initMMU_setup(void)^~~~~~~~~cc1: all warnings being treated as errors
Can't it be static instead?
$ git grep -n MMU_setup arch/powerpc/mm/init_32.c:102:void __init MMU_setup(void) arch/powerpc/mm/init_32.c:135: MMU_setup();
cheers
PATCH v2 15/21 powerpc: Make function MMU_setup static
In reply to Mathieu MalaterreSince function MMU_setup is not meant to be exported, change the signature to static. Fix warning (treated as error with W=1):
CC kernel/sys.oarch/powerpc/mm/init_32.c:102:13: error: no previous prototype for ‘MMU_setup’ [-Werror=missing-prototypes]
void __init MMU_setup(void)
^~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/mm/init_32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/powerpc/mm/init_32.c +1 -1
@@ -99,7 +99,7 @@ unsigned long __max_low_memory = MAX_LOW_MEM;/* * Check for command-line options that affect what MMU_init will do. */ -void __init MMU_setup (void ) +static void __init MMU_setup (void ) {/* Check for nobats option (used in mapin_ram). */ if (strstr (boot_command_line,"nobats" )) {
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [v2,15/21] powerpc: Make function MMU_setup static
Quoted 10 lines · Mathieu Malaterre
Since function
MMU_setupis not meant to be exported, change the signature tostatic. Fix warning (treated as error with W=1):CC
kernel/sys.oarch/powerpc/mm/init_32.c:102:13: error: no previous prototype for ‘MMU_setup’ [-Werror=missing-prototypes]
void__initMMU_setup(void)^~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/d15a261d876da3267c8c706ef21e7f
cheers
PATCH 16/21 powerpc: Add missing prototype for init_IRQ
In reply to Mathieu MalaterreA function init_IRQ was added without a prototype declared in header irq.h. Fix the following warning (treated as error in W=1):
CC arch/powerpc/kernel/irq.oarch/powerpc/kernel/irq.c:662:13: error: no previous prototype for ‘init_IRQ’ [-Werror=missing-prototypes]
void __init init_IRQ(void)
^~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/irq.h | 1 + 1 file changed, 1 insertion(+)
arch/powerpc/include/asm/irq.h +1 -0
@@ -66,6 +66,7 @@ extern void irq_ctx_init(void);extern void call_do_softirq (struct thread_info *tp);extern void call_do_irq (struct pt_regs *regs,struct thread_info *tp);extern void do_IRQ (struct pt_regs *regs); +extern void __init init_IRQ (void );extern void __do_irq (struct pt_regs *regs);int irq_choose_cpu (const struct cpumask *mask);
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [16/21] powerpc: Add missing prototype for init_IRQ
Quoted 10 lines · Mathieu Malaterre
A function
init_IRQwas added without a prototype declared in headerirq.h. Fix the following warning (treated as error in W=1):CC
arch/powerpc/kernel/irq.oarch/powerpc/kernel/irq.c:662:13: error: no previous prototype for ‘init_IRQ’ [-Werror=missing-prototypes]
void__initinit_IRQ(void)^~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/23a6d8b9634897add6ebff32372f34
cheers
PATCH 17/21 powerpc: Add missing prototype for sys_debug_setcontext
In reply to Mathieu MalaterreIn commit 81e7009ea46c ("powerpc: merge ppc signal.c and ppc64 signal32.c") the function sys_debug_setcontext was added without a prototype.
Fix compilation warning (treated as error in W=1):
CC arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
int sys_debug_setcontext(struct ucontext __user *ctx,
^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/asm-prototypes.h | 4 ++++ 1 file changed, 4 insertions(+)
arch/powerpc/include/asm/asm-prototypes.h +4 -0
@@ -89,6 +89,10 @@ int sys_swapcontext(struct ucontext __user *old_ctx,long sys_swapcontext (struct ucontext __user *old_ctx,struct ucontext __user *new_ctx,int ctx_size,int r6,int r7,int r8,struct pt_regs *regs); +int sys_debug_setcontext (struct ucontext __user *ctx, +int ndbg,struct sig_dbg_op __user *dbg, +int r6,int r7,int r8, +struct pt_regs *regs);#endif long sys_switch_endian (void ); notrace unsigned int __check_irq_replay(void);
Signature · 1 line
2.11.0
Michael Ellerman
reviewerRe: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext
Quoted 1 line · Mathieu Malaterre
In commit
81e7009ea46c("powerpc: merge ppcsignal.cand ppc64 signal32.c=
")
Quoted 6 lines
the function
sys_debug_setcontextwas added without a prototype.Fix compilation warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for =
=E2=80=98sys_debug_setcontext=E2=80=99 [-Werror=3Dmissing-prototypes]
Quoted 3 lines
int sys_debug_setcontext(struct ucontext
__user*ctx,^~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
This one should actually be using the SYSCALL_DEFINE syntax, so that it can be used with CONFIG_FTRACE_SYSCALLS.
See eg. our mmap:
SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
unsigned long, prot, unsigned long, flags, unsigned long, fd, off_t, offset)
{
return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
}
We probably still need this patch, but I'm not entirely sure because the SYSCALL_DEFINE macro does all sorts of shenanigans.
cheers
Re: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext
On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <[email protected]> wrot= e:
Quoted 3 lines
Quoted 1 line · Mathieu Malaterre
In commit
81e7009ea46c("powerpc: merge ppcsignal.cand ppc64 signal32.=
c")
Quoted 6 lines
Quoted 6 lines
the function
sys_debug_setcontextwas added without a prototype.Fix compilation warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for=
=E2=80=98sys_debug_setcontext=E2=80=99 [-Werror=3Dmissing-prototypes]
Quoted 12 lines
Quoted 3 lines
int sys_debug_setcontext(struct ucontext
__user*ctx,^~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
This one should actually be using the
SYSCALL_DEFINEsyntax, so that it can be used withCONFIG_FTRACE_SYSCALLS.See eg. our mmap:
SYSCALL_DEFINE6(mmap, unsigned long, addr,
size_t, len,
unsigned long, prot, unsigned long, flags, unsigned long, fd,off_t, offset)
Quoted 7 lines
{ return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);}
We probably still need this patch, but I'm not entirely sure because the
SYSCALL_DEFINEmacro does all sorts of shenanigans.
I see. Could you please drop this patch then. The patch does not look that trivial anymore. I'll need to dig a bit more on how to do the syscall stuff with a 7 params function.
Thanks
Michael Ellerman
reviewerRe: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext
Quoted 1 line · Mathieu Malaterre
On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <[email protected]> wr=
ote:
Quoted 3 lines
Quoted 3 lines
Quoted 1 line · Mathieu Malaterre
In commit
81e7009ea46c("powerpc: merge ppcsignal.cand ppc64 signal32=
.c")
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
the function
sys_debug_setcontextwas added without a prototype.Fix compilation warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype fo=
r =E2=80=98sys_debug_setcontext=E2=80=99 [-Werror=3Dmissing-prototypes]
Quoted 17 lines
Quoted 17 lines
Quoted 3 lines
int sys_debug_setcontext(struct ucontext
__user*ctx,^~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
This one should actually be using the
SYSCALL_DEFINEsyntax, so that it can be used withCONFIG_FTRACE_SYSCALLS.See eg. our mmap:
SYSCALL_DEFINE6(mmap, unsigned long, addr,
size_t, len,
unsigned long, prot, unsigned long, flags, unsigned long, fd,off_t, offset){ return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);}
Quoted 6 lines
Quoted 2 lines
We probably still need this patch, but I'm not entirely sure because the
SYSCALL_DEFINEmacro does all sorts of shenanigans.I see. Could you please drop this patch then. The patch does not look that trivial anymore. I'll need to dig a bit more on how to do the syscall stuff with a 7 params function.
Ergh, yuck, seems we're the first suckers to need do that.
I think I'll take this patch for now, it's still good for now at least, and then the SYSCALL_DEFINE stuff can be an addition.
cheers
Re: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext
Michael,
On Thu, Mar 8, 2018 at 11:36 AM, Michael Ellerman <[email protected]> wrot= e:
Quoted 3 lines
Quoted 1 line · Mathieu Malaterre
On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <[email protected]> w=
rote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line · Mathieu Malaterre
In commit
81e7009ea46c("powerpc: merge ppcsignal.cand ppc64 signal3=
2.c")
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
the function
sys_debug_setcontextwas added without a prototype.Fix compilation warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype f=
or =E2=80=98sys_debug_setcontext=E2=80=99 [-Werror=3Dmissing-prototypes]
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
int sys_debug_setcontext(struct ucontext
__user*ctx,^~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
This one should actually be using the
SYSCALL_DEFINEsyntax, so that it can be used withCONFIG_FTRACE_SYSCALLS.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
See eg. our mmap:
SYSCALL_DEFINE6(mmap, unsigned long, addr,
size_t, len,
unsigned long, prot, unsigned long, flags, unsigned long, fd,off_t, offset)
{
returndo_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT)=
;
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
}We probably still need this patch, but I'm not entirely sure because th=
e
Quoted 10 lines
Quoted 5 lines
Quoted 1 line
SYSCALL_DEFINEmacro does all sorts of shenanigans.I see. Could you please drop this patch then. The patch does not look that trivial anymore. I'll need to dig a bit more on how to do the syscall stuff with a 7 params function.
Ergh, yuck, seems we're the first suckers to need do that.
I think I'll take this patch for now, it's still good for now at least, and then the
SYSCALL_DEFINEstuff can be an addition.
Just to close the loop (for later reference). Here is what Al Viro told me about this function
[begin quote]
int sys_debug_setcontext(struct ucontext __user *ctx,
int ndbg, struct sig_dbg_op __user *dbg, int r6, int r7, int r8,
struct pt_regs *regs)
can't be converted to SYSCALL_DEFINE... not because it's a 7-argument syscall (it isn't); the problem is that the last argument here does *not* come from userland. What it really is trying to be is 3-argument syscall:
SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
int, ndbg,
struct sig_dbg_op __user *, dbg)
with no dummy r6/r7/r8. The thing is, ppc dispatcher combines the "pass 6 arguments" and "pass pt_regs *" by passing both.
debug_setcontext(), swapcontext(), sigreturn() and rt_sigreturn() are, AFAICS, the only ppc syscalls that make use of that argument. All of those are relying upon regs =3D=3D current_pt_regs() =3D=3D current->thread= .regs,
[...]
But in any case, these are not 7-argument syscalls - debug_setcontext(2) and swapcontext(2) are 3-argument and sigreturn()/rt_sigreturn() have no arguments at all.
[end quote]
Sorry about the wrong analysis in the number of arguments count. I believe commit 0d60619e1c0ca (powerpc/next) should be just fine for now.
2cts
Michael Ellerman
reviewerRe: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext
Hi Mathieu,
Quoted 1 line · Mathieu Malaterre
On Thu, Mar 8, 2018 at 11:36 AM, Michael Ellerman <[email protected]> wr=
ote:
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · Mathieu Malaterre
On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <[email protected]> =
Quoted 2 lines · wrote:
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · Mathieu Malaterre
In commit
81e7009ea46c("powerpc: merge ppcsignal.cand ppc64 signal=
32.c")
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
the function
sys_debug_setcontextwas added without a prototype.Fix compilation warning (treated as error in W=3D1):
CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype =
for =E2=80=98sys_debug_setcontext=E2=80=99 [-Werror=3Dmissing-prototypes]
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
int sys_debug_setcontext(struct ucontext
__user*ctx,^~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
This one should actually be using the
SYSCALL_DEFINEsyntax, so that it can be used withCONFIG_FTRACE_SYSCALLS.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
See eg. our mmap:
SYSCALL_DEFINE6(mmap, unsigned long, addr,
size_t, len,
unsigned long, prot, unsigned long, flags, unsigned long, fd,off_t, offset)
{
return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT=
);
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
}We probably still need this patch, but I'm not entirely sure because t=
he
Quoted 30 lines
Quoted 10 lines
Quoted 5 lines
Quoted 1 line
SYSCALL_DEFINEmacro does all sorts of shenanigans.I see. Could you please drop this patch then. The patch does not look that trivial anymore. I'll need to dig a bit more on how to do the syscall stuff with a 7 params function.
Ergh, yuck, seems we're the first suckers to need do that.
I think I'll take this patch for now, it's still good for now at least, and then the
SYSCALL_DEFINEstuff can be an addition.Just to close the loop (for later reference). Here is what Al Viro told me about this function
[begin quote]
int sys_debug_setcontext(struct ucontext__user*ctx,
int ndbg, structsig_dbg_op__user*dbg, int r6, int r7, int r8,
structpt_regs*regs)can't be converted to
SYSCALL_DEFINE... not because it's a 7-argument syscall (it isn't); the problem is that the last argument here does *not* come from userland. What it really is trying to be is 3-argument syscall:
SYSCALL_DEFINE3(debug_setcontext, struct ucontext__user*, ctx,
int, ndbg,
structsig_dbg_op__user*, dbg)
with no dummy r6/r7/r8. The thing is, ppc dispatcher combines the "pass 6 arguments" and "passpt_regs*" by passing both.
Quoted 3 lines
debug_setcontext(),swapcontext(),sigreturn()andrt_sigreturn()are, AFAICS, the only ppc syscalls that make use of that argument. All of those are relying upon regs =3D=3Dcurrent_pt_regs()=3D=3D current->thre=
ad.regs,
Quoted 5 lines
[...]
But in any case, these are not 7-argument syscalls -debug_setcontext(2)and swapcontext(2) are 3-argument and sigreturn()/rt_sigreturn() have no arguments at all.
[end quote]
Awesome follow-up thanks.
I should have realised that's what the code was doing, but I didn't look closely enough. Apologies to Al for having to stare at our horrible signal code again :)
We should really clean all that up, I can't see any reason those syscalls aren't using current_pt_regs().
That would then mean we could use SYSCALL_DEFINE.
I wrote that down so we don't (might not) forget:
https://github.com/linuxppc/linux/issues/131
Quoted 3 lines
Sorry about the wrong analysis in the number of arguments count. I believe commit
0d60619e1c0ca(powerpc/next) should be just fine for now.
No worries.
cheers
Michael Ellerman
AppliedRe: [17/21] powerpc: Add missing prototype for sys_debug_setcontext
In reply to Mathieu MalaterreQuoted 5 lines · Mathieu Malaterre
In commit
81e7009ea46c("powerpc: merge ppcsignal.cand ppc64signal32.c") the functionsys_debug_setcontextwas added without a prototype.Fix compilation warning (treated as error in W=1):
Quoted 7 lines
CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
int sys_debug_setcontext(struct ucontext__user*ctx,^~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/0d60619e1c0ca20eb2610361034992
cheers
PATCH 18/21 powerpc: Add missing prototypes for sys_sigreturn & sys_rt_sigreturn
In reply to Mathieu MalaterreTwo functions did not have a prototype defined in signal.h header. Fix the following two warnings (treated as errors in W=1):
CC arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1135:6: error: no previous prototype for ‘sys_rt_sigreturn’ [-Werror=missing-prototypes]
long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
^~~~~~~~~~~~~~~~
arch/powerpc/kernel/signal_32.c:1422:6: error: no previous prototype for ‘sys_sigreturn’ [-Werror=missing-prototypes]
long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
^~~~~~~~~~~~~cc1: all warnings being treated as errors
--- arch/powerpc/kernel/signal.h | 5 +++++ 1 file changed, 5 insertions(+)
arch/powerpc/kernel/signal.h +5 -0
@@ -49,6 +49,11 @@ extern int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,#else /* CONFIG_PPC64 */ +extern long sys_rt_sigreturn (int r3,int r4,int r5,int r6,int r7,int r8, +struct pt_regs *regs); +extern long sys_sigreturn (int r3,int r4,int r5,int r6,int r7,int r8, +struct pt_regs *regs); +static inline int handle_rt_signal64 (struct ksignal *ksig,sigset_t *set,struct task_struct *tsk) {
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [18/21] powerpc: Add missing prototypes for sys_sigreturn & sys_rt_sigreturn
Quoted 6 lines · Mathieu Malaterre
Two functions did not have a prototype defined in
signal.hheader. Fix the following two warnings (treated as errors in W=1):CC
arch/powerpc/kernel/signal_32.oarch/powerpc/kernel/signal_32.c:1135:6: error: no previous prototype for ‘sys_rt_sigreturn’ [-Werror=missing-prototypes]
long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
Quoted 7 lines
^~~~~~~~~~~~~~~~ arch/powerpc/kernel/signal_32.c:1422:6: error: no previous prototype for ‘sys_sigreturn’ [-Werror=missing-prototypes] long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8, ^~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/b53875c4b4f212a7a8e505e2d10635
cheers
PATCH 19/21 powerpc: Add missing prototypes for hw_breakpoint_handler & arch_unregister_hw_breakpoint
In reply to Mathieu MalaterreIn commit 5aae8a537080 ("powerpc, hw_breakpoints: Implement hw_breakpoints for 64-bit server processors") function hw_breakpoint_handler and arch_unregister_hw_breakpoint were added without function prototypes in hw_breakpoint.h header.
Fix the following warning(s) (treated as error in W=1):
AR init/built-in.oarch/powerpc/kernel/hw_breakpoint.c:106:6: error: no previous prototype for ‘arch_unregister_hw_breakpoint’ [-Werror=missing-prototypes]
void arch_unregister_hw_breakpoint(struct perf_event *bp)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/hw_breakpoint.c:209:5: error: no previous prototype for ‘hw_breakpoint_handler’ [-Werror=missing-prototypes]
int hw_breakpoint_handler(struct die_args *args)
^~~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/hw_breakpoint.h | 2 ++ 1 file changed, 2 insertions(+)
arch/powerpc/include/asm/hw_breakpoint.h +2 -0
@@ -66,6 +66,7 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused, unsigned long val, void *data);int arch_install_hw_breakpoint (struct perf_event *bp);void arch_uninstall_hw_breakpoint (struct perf_event *bp); +void arch_unregister_hw_breakpoint (struct perf_event *bp);void hw_breakpoint_pmu_read (struct perf_event *bp);extern void flush_ptrace_hw_breakpoint (struct task_struct *tsk);
@@ -82,6 +83,7 @@ static inline void hw_breakpoint_disable(void) __set_breakpoint(&brk); }extern void thread_change_pc (struct task_struct *tsk,struct pt_regs *regs); +int hw_breakpoint_handler (struct die_args *args);#else /* CONFIG_HAVE_HW_BREAKPOINT */ static inline void hw_breakpoint_disable (void ) { }
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [19/21] powerpc: Add missing prototypes for hw_breakpoint_handler & arch_unregister_hw_breakpoint
Quoted 10 lines · Mathieu Malaterre
In commit
5aae8a537080("powerpc,hw_breakpoints: Implementhw_breakpointsfor 64-bit server processors") functionhw_breakpoint_handlerandarch_unregister_hw_breakpointwere added without function prototypes inhw_breakpoint.hheader.Fix the following warning(s) (treated as error in W=1):
AR
init/built-in.oarch/powerpc/kernel/hw_breakpoint.c:106:6: error: no previous prototype for ‘arch_unregister_hw_breakpoint’ [-Werror=missing-prototypes]
voidarch_unregister_hw_breakpoint(struct perf_event *bp)
Quoted 7 lines
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/powerpc/kernel/hw_breakpoint.c:209:5: error: no previous prototype for ‘hw_breakpoint_handler’ [-Werror=missing-prototypes] int hw_breakpoint_handler(struct die_args *args) ^~~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/b0d876da1d1cd33a1c28049512a136
cheers
PATCH 20/21 powerpc: Add missing prototypes for ppc_select & ppc_fadvise64_64
In reply to Mathieu MalaterreAdd missing prototypes for ppc_select & ppc_fadvise64_64 to header asm-prototypes.h. Fix the following warnings (treated as errors in W=1)
CC arch/powerpc/kernel/syscalls.oarch/powerpc/kernel/syscalls.c:87:1: error: no previous prototype for ‘ppc_select’ [-Werror=missing-prototypes]
ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct timeval __user *tvp) ^~~~~~~~~~arch/powerpc/kernel/syscalls.c:119:6: error: no previous prototype for ‘ppc_fadvise64_64’ [-Werror=missing-prototypes]
long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--- arch/powerpc/include/asm/asm-prototypes.h | 4 ++++ 1 file changed, 4 insertions(+)
arch/powerpc/include/asm/asm-prototypes.h +4 -0
@@ -93,7 +93,11 @@ int sys_debug_setcontext(struct ucontext __user *ctx, int ndbg, struct sig_dbg_op __user *dbg, int r6, int r7, int r8, struct pt_regs *regs); +int +ppc_select (int n,fd_set __user *inp,fd_set __user *outp,fd_set __user *exp,struct timeval __user *tvp);#endif +long ppc_fadvise64_64 (int fd,int advice,u32 offset_high,u32 offset_low, +u32 len_high,u32 len_low);long sys_switch_endian (void ); notrace unsigned int __check_irq_replay(void); void notrace restore_interrupts(void);
Signature · 1 line
2.11.0
Michael Ellerman
AppliedRe: [20/21] powerpc: Add missing prototypes for ppc_select & ppc_fadvise64_64
Quoted 9 lines · Mathieu Malaterre
Add missing prototypes for
ppc_select&ppc_fadvise64_64to headerasm-prototypes.h. Fix the following warnings (treated as errors in W=1)CC
arch/powerpc/kernel/syscalls.oarch/powerpc/kernel/syscalls.c:87:1: error: no previous prototype for ‘ppc_select’ [-Werror=missing-prototypes]
ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct timeval __user *tvp)^~~~~~~~~~arch/powerpc/kernel/syscalls.c:119:6: error: no previous prototype for ‘ppc_fadvise64_64’ [-Werror=missing-prototypes]
long ppc_fadvise64_64(int fd, int advice, u32offset_high, u32offset_low,
Quoted 4 lines
^~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/bf7fb32dd5fc8a6bd25aaab05d3acd
cheers
PATCH 21/21 powerpc: Add missing prototypes in setup_32.c
In reply to Mathieu MalaterreThis commit add the prototypes for the following function:
- early_init
- machine_init
- ppc_setup_l2cr
- ppc_setup_l3cr
- ppc_init
the other missing ones were already added in setup.h previously. Fix the following warnings (treated as error in W=1):
AR init/built-in.oarch/powerpc/kernel/setup_32.c:68:30: error: no previous prototype for ‘early_init’ [-Werror=missing-prototypes]
notrace unsigned long __init early_init(unsigned long dt_ptr)
^~~~~~~~~~
arch/powerpc/kernel/setup_32.c:99:21: error: no previous prototype for ‘machine_init’ [-Werror=missing-prototypes]
notrace void __init machine_init(u64 dt_ptr)
^~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:124:12: error: no previous prototype for ‘ppc_setup_l2cr’ [-Werror=missing-prototypes]
int __init ppc_setup_l2cr(char *str)
^~~~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:137:12: error: no previous prototype for ‘ppc_setup_l3cr’ [-Werror=missing-prototypes]
int __init ppc_setup_l3cr(char *str)
^~~~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:183:12: error: no previous prototype for ‘ppc_init’ [-Werror=missing-prototypes]
int __init ppc_init(void)
^~~~~~~~
arch/powerpc/kernel/setup_32.c:198:13: error: no previous prototype for ‘irqstack_early_init’ [-Werror=missing-prototypes]
void __init irqstack_early_init(void)
^~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:238:13: error: no previous prototype for ‘setup_power_save’ [-Werror=missing-prototypes]
void __init setup_power_save(void)
^~~~~~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:253:13: error: no previous prototype for ‘initialize_cache_info’ [-Werror=missing-prototypes]
__init void initialize_cache_info(void)
^~~~~~~~~~~~~~~~~~~~~cc1: all warnings being treated as errors
--- arch/powerpc/kernel/setup.h | 5 +++++ arch/powerpc/kernel/setup_32.c | 2 ++ 2 files changed, 7 insertions(+)
arch/powerpc/kernel/setup.h +5 -0
@@ -17,6 +17,11 @@ void irqstack_early_init(void);#ifdef CONFIG_PPC32 void setup_power_save (void ); +unsigned long __init early_init (unsigned long dt_ptr); +void __init machine_init (u64 dt_ptr); +int __init ppc_setup_l2cr (char *str); +int __init ppc_setup_l3cr (char *str); +int __init ppc_init (void );#else static inline void setup_power_save (void ) { };#endif
arch/powerpc/kernel/setup_32.c +2 -0
@@ -40,6 +40,8 @@#include <asm/code-patching.h> #include <asm/cpu_has_feature.h> +#include "setup.h" + #define DBG(fmt...)extern void bootx_init (unsigned long r4,unsigned long phys);
Signature · 1 line
2.11.0
Michael Ellerman
reviewerRe: [PATCH 21/21] powerpc: Add missing prototypes in setup_32.c
Quoted 4 lines · Mathieu Malaterre
This commit add the prototypes for the following function:
-
early_init
-machine_init
I'd rather these were in asm-prototypes.h, it's the header for functions called from asm. I made that change.
Quoted 3 lines
-
ppc_setup_l2cr
-ppc_setup_l3cr
-ppc_init
These three can just be static. So I did that.
cheers
Michael Ellerman
AppliedRe: [21/21] powerpc: Add missing prototypes in setup_32.c
In reply to Mathieu MalaterreQuoted 36 lines · Mathieu Malaterre
This commit add the prototypes for the following function:
-
early_init
-machine_init
-ppc_setup_l2cr
-ppc_setup_l3cr
-ppc_initthe other missing ones were already added in
setup.hpreviously. Fix the following warnings (treated as error in W=1):AR
init/built-in.oarch/powerpc/kernel/setup_32.c:68:30: error: no previous prototype for ‘early_init’ [-Werror=missing-prototypes]
notrace unsigned long__initearly_init(unsigned long dt_ptr)^~~~~~~~~~ arch/powerpc/kernel/setup_32.c:99:21: error: no previous prototype for ‘machine_init’ [-Werror=missing-prototypes] notrace void __init machine_init(u64 dt_ptr) ^~~~~~~~~~~~ arch/powerpc/kernel/setup_32.c:124:12: error: no previous prototype for ‘ppc_setup_l2cr’ [-Werror=missing-prototypes] int __init ppc_setup_l2cr(char *str) ^~~~~~~~~~~~~~ arch/powerpc/kernel/setup_32.c:137:12: error: no previous prototype for ‘ppc_setup_l3cr’ [-Werror=missing-prototypes] int __init ppc_setup_l3cr(char *str) ^~~~~~~~~~~~~~ arch/powerpc/kernel/setup_32.c:183:12: error: no previous prototype for ‘ppc_init’ [-Werror=missing-prototypes] int __init ppc_init(void) ^~~~~~~~ arch/powerpc/kernel/setup_32.c:198:13: error: no previous prototype for ‘irqstack_early_init’ [-Werror=missing-prototypes] void __init irqstack_early_init(void) ^~~~~~~~~~~~~~~~~~~ arch/powerpc/kernel/setup_32.c:238:13: error: no previous prototype for ‘setup_power_save’ [-Werror=missing-prototypes] void __init setup_power_save(void) ^~~~~~~~~~~~~~~~ arch/powerpc/kernel/setup_32.c:253:13: error: no previous prototype for ‘initialize_cache_info’ [-Werror=missing-prototypes] __init void initialize_cache_info(void) ^~~~~~~~~~~~~~~~~~~~~
Quoted 3 lines
cc1: all warnings being treated as errors
Signed-off-by Mathieu Malaterre <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/e82d70cf965072a3872ea97b7d8df4
cheers