| Message ID | 20190517064922.22743-1-yamada.masahiro@socionext.com |
|---|---|
| State | Accepted |
| Commit | bf2f1eeed0b5461ecda385f1716c2b65add54579 |
| Headers | show |
| Series | s390: add unreachable() to dump_fault_info() to fix -Wmaybe-uninitialized | expand |
On (05/17/19 15:49), Masahiro Yamada wrote: [..] > @@ -211,6 +211,8 @@ static void dump_fault_info(struct pt_regs *regs) > asce = S390_lowcore.kernel_asce; > pr_cont("kernel "); > break; > + default: > + unreachable(); > } > pr_cont("ASCE.\n"); > dump_pagetable(asce, regs->int_parm_long & __FAIL_ADDR_MASK); Or have default fault type and asce? Just an idea. --- diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c index c220399ae196..876d71c31894 100644 --- a/arch/s390/mm/fault.c +++ b/arch/s390/mm/fault.c @@ -174,7 +174,8 @@ static void dump_pagetable(unsigned long asce, unsigned long address) static void dump_fault_info(struct pt_regs *regs) { - unsigned long asce; + unsigned long asce = S390_lowcore.kernel_asce; + char *type = "kernel "; pr_alert("Failing address: %016lx TEID: %016lx\n", regs->int_parm_long & __FAIL_ADDR_MASK, regs->int_parm_long); @@ -197,22 +198,18 @@ static void dump_fault_info(struct pt_regs *regs) switch (get_fault_type(regs)) { case USER_FAULT: asce = S390_lowcore.user_asce; - pr_cont("user "); + type = "user "; break; case VDSO_FAULT: asce = S390_lowcore.vdso_asce; - pr_cont("vdso "); + type = "vdso "; break; case GMAP_FAULT: asce = ((struct gmap *) S390_lowcore.gmap)->asce; - pr_cont("gmap "); - break; - case KERNEL_FAULT: - asce = S390_lowcore.kernel_asce; - pr_cont("kernel "); + type = "gmap "; break; } - pr_cont("ASCE.\n"); + pr_cont("%s ASCE.\n", type); dump_pagetable(asce, regs->int_parm_long & __FAIL_ADDR_MASK); }
On Fri, 17 May 2019 15:49:22 +0900 Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > When CONFIG_OPTIMIZE_INLINING is enabled for s390, I see this warning: > > arch/s390/mm/fault.c:127:15: warning: 'asce' may be used uninitialized in this function [-Wmaybe-uninitialized] > switch (asce & _ASCE_TYPE_MASK) { > arch/s390/mm/fault.c:177:16: note: 'asce' was declared here > unsigned long asce; > ^~~~ > > If get_fault_type() is not inlined, the compiler cannot deduce that > all the possible paths in the 'switch' statement are covered. > > Of course, we could mark get_fault_type() as __always_inline to get > back the original behavior, but I do not think it sensible to force > inlining just for the purpose of suppressing the warning. Since this > is just a matter of warning, I want to keep as much room for compiler > optimization as possible. > > I added unreachable() to teach the compiler that the 'default' label > is unreachable. > > I got rid of the 'inline' marker. Even without the 'inline' hint, > the compiler inlines functions based on its inlining heuristic. > > Fixes: 9012d011660e ("compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING") > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Added to our internal tree and I will add it to s390/linux soon. Thanks. > --- > > arch/s390/mm/fault.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c > index c220399ae196..91ce03fd0c84 100644 > --- a/arch/s390/mm/fault.c > +++ b/arch/s390/mm/fault.c > @@ -85,7 +85,7 @@ static inline int notify_page_fault(struct pt_regs *regs) > * Find out which address space caused the exception. > * Access register mode is impossible, ignore space == 3. > */ > -static inline enum fault_type get_fault_type(struct pt_regs *regs) > +static enum fault_type get_fault_type(struct pt_regs *regs) > { > unsigned long trans_exc_code; > > @@ -211,6 +211,8 @@ static void dump_fault_info(struct pt_regs *regs) > asce = S390_lowcore.kernel_asce; > pr_cont("kernel "); > break; > + default: > + unreachable(); > } > pr_cont("ASCE.\n"); > dump_pagetable(asce, regs->int_parm_long & __FAIL_ADDR_MASK); -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c index c220399ae196..91ce03fd0c84 100644 --- a/arch/s390/mm/fault.c +++ b/arch/s390/mm/fault.c @@ -85,7 +85,7 @@ static inline int notify_page_fault(struct pt_regs *regs) * Find out which address space caused the exception. * Access register mode is impossible, ignore space == 3. */ -static inline enum fault_type get_fault_type(struct pt_regs *regs) +static enum fault_type get_fault_type(struct pt_regs *regs) { unsigned long trans_exc_code; @@ -211,6 +211,8 @@ static void dump_fault_info(struct pt_regs *regs) asce = S390_lowcore.kernel_asce; pr_cont("kernel "); break; + default: + unreachable(); } pr_cont("ASCE.\n"); dump_pagetable(asce, regs->int_parm_long & __FAIL_ADDR_MASK);
When CONFIG_OPTIMIZE_INLINING is enabled for s390, I see this warning: arch/s390/mm/fault.c:127:15: warning: 'asce' may be used uninitialized in this function [-Wmaybe-uninitialized] switch (asce & _ASCE_TYPE_MASK) { arch/s390/mm/fault.c:177:16: note: 'asce' was declared here unsigned long asce; ^~~~ If get_fault_type() is not inlined, the compiler cannot deduce that all the possible paths in the 'switch' statement are covered. Of course, we could mark get_fault_type() as __always_inline to get back the original behavior, but I do not think it sensible to force inlining just for the purpose of suppressing the warning. Since this is just a matter of warning, I want to keep as much room for compiler optimization as possible. I added unreachable() to teach the compiler that the 'default' label is unreachable. I got rid of the 'inline' marker. Even without the 'inline' hint, the compiler inlines functions based on its inlining heuristic. Fixes: 9012d011660e ("compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING") Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- arch/s390/mm/fault.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 2.17.1