powerpc/fadump: rename cpu_online_mask member of struct fadump_crash_info_header
authorRasmus Villemoes <linux@rasmusvillemoes.dk>
Wed, 20 Jan 2016 23:00:13 +0000 (15:00 -0800)
committerLinus Torvalds <torvalds@linux-foundation.org>
Thu, 21 Jan 2016 01:09:18 +0000 (17:09 -0800)
The four cpumasks cpu_{possible,online,present,active}_bits are exposed
readonly via the corresponding const variables cpu_xyz_mask.  But they are
also accessible for arbitrary writing via the exposed functions
set_cpu_xyz.  There's quite a bit of code throughout the kernel which
iterates over or otherwise accesses these bitmaps, and having the access
go via the cpu_xyz_mask variables is nowadays [1] simply a useless
indirection.

It may be that any problem in CS can be solved by an extra level of
indirection, but that doesn't mean every extra indirection solves a
problem.  In this case, it even necessitates some minor ugliness (see
4/6).

Patch 1/6 is new in v2, and fixes a build failure on ppc by renaming a
struct member, to avoid problems when the identifier cpu_online_mask
becomes a macro later in the series.  The next four patches eliminate the
cpu_xyz_mask variables by simply exposing the actual bitmaps, after
renaming them to discourage direct access - that still happens through
cpu_xyz_mask, which are now simply macros with the same type and value as
they used to have.

After that, there's no longer any reason to have the setter functions be
out-of-line: The boolean parameter is almost always a literal true or
false, so by making them static inlines they will usually compile to one
or two instructions.

For a defconfig build on x86_64, bloat-o-meter says we save ~3000 bytes.
We also save a little stack (stackdelta says 127 functions have a 16 byte
smaller stack frame, while two grow by that amount).  Mostly because, when
iterating over the mask, gcc typically loads the value of cpu_xyz_mask
into a callee-saved register and from there into %rdi before each
find_next_bit call - now it can just load the appropriate immediate
address into %rdi before each call.

[1] See Rusty's kind explanation
http://thread.gmane.org/gmane.linux.kernel/2047078/focus=2047722 for
some historic context.

This patch (of 6):

As preparation for eliminating the indirect access to the various global
cpu_*_bits bitmaps via the pointer variables cpu_*_mask, rename the
cpu_online_mask member of struct fadump_crash_info_header to simply
online_mask, thus allowing cpu_online_mask to become a macro.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
arch/powerpc/include/asm/fadump.h
arch/powerpc/kernel/fadump.c

index 493e72f..b4407d0 100644 (file)
@@ -191,7 +191,7 @@ struct fadump_crash_info_header {
        u64             elfcorehdr_addr;
        u32             crashing_cpu;
        struct pt_regs  regs;
-       struct cpumask  cpu_online_mask;
+       struct cpumask  online_mask;
 };
 
 /* Crash memory ranges */
index 26d091a..3cb3b02 100644 (file)
@@ -415,7 +415,7 @@ void crash_fadump(struct pt_regs *regs, const char *str)
        else
                ppc_save_regs(&fdh->regs);
 
-       fdh->cpu_online_mask = *cpu_online_mask;
+       fdh->online_mask = *cpu_online_mask;
 
        /* Call ibm,os-term rtas call to trigger firmware assisted dump */
        rtas_os_term((char *)str);
@@ -646,7 +646,7 @@ static int __init fadump_build_cpu_notes(const struct fadump_mem_struct *fdm)
                }
                /* Lower 4 bytes of reg_value contains logical cpu id */
                cpu = be64_to_cpu(reg_entry->reg_value) & FADUMP_CPU_ID_MASK;
-               if (fdh && !cpumask_test_cpu(cpu, &fdh->cpu_online_mask)) {
+               if (fdh && !cpumask_test_cpu(cpu, &fdh->online_mask)) {
                        SKIP_TO_NEXT_CPU(reg_entry);
                        continue;
                }