__pcpu: gcc -Wredundant-decls

[rlibby]

Summary

I'm sorry for the long text following. The problem is ultimately pretty simple and I'm happy to solve this however seems best, or to leave it to you, kib. This is the last patch of about 10 to get amd64-gcc buildkernel working again.

The extern struct pcpu __pcpu[]; in sys/amd64/include/counter.h is now visible in sys/amd64/amd64/pmap.c via the include in sys/sys/vmmeter.h (new with the counter changes in r317061). pmap.c has its own extern struct pcpu __pcpu[]; declaration. gcc detects this and issues a warning because we use -Wredundant-decls.

Three solutions occur to me.

1. Simple one-line hack, delete the pmap.c extern decl. Nice and easy, but unfortunately this codifies reliance on pollution from counter.h, which feels weird.
2. Declare extern struct pcpu __pcpu[]; in sys/*/include/pcpu.h, delete all other extern declarations. This solves the problem but exposes the symbol even more widely than now with counter.h.
3. Create new headers at sys/*/include/_pcpu.h (?) and put the declaration there. Replace all other extern declarations with an inclusion of _pcpu.h. This avoids making the namespace pollution any worse than with counter.h, but it seems like a pretty heavyweight solution for a single decl.
4. Something else?

The attached patch implements option 1.

Test Plan

gcc

[kib]

I would strongly prefer #2. I believe that your patch makes pmap code rely on the hack from counter.h which is wrong both for counter.h and for pmap.c.

I do not see any problem with exposing this definition for kernel.

[rlibby]

In D11666#241708, @kib wrote:

I would strongly prefer #2. I believe that your patch makes pmap code rely on the hack from counter.h which is wrong both for counter.h and for pmap.c.

I do not see any problem with exposing this definition for kernel.

I started to try this, but I hit a problem. Now I see, sys/$arch/include/pcpu.h can't declare extern struct pcpu __pcpu[]; because we don't have a complete type definition for struct pcpu in those headers (we haven't seen the machine-independent bits yet from sys/sys/pcpu.h).

However, I think I found an okay workaround. The pmap.c usage is in pmap_bootstrap, and it looks like by that point we have already bootstrapped pcpu (hammer_time sets up pcpu and uses PCPU_SET before calling getmemsize/pmap_bootstrap). Therefore, I think we can just use PCPU_SET(foo, bar) instead of __pcpu[0].foo = bar in pmap_bootstrap. With that, we can remove both the extern declration of __pcpu and the direct references to it from pmap.c.

I'll post a patch in a little bit. I also noticed a few other places that don't need to be using __pcpu directly.

[kib]

In D11666#242001, @rlibby wrote:

However, I think I found an okay workaround. The pmap.c usage is in pmap_bootstrap, and it looks like by that point we have already bootstrapped pcpu (hammer_time sets up pcpu and uses PCPU_SET before calling getmemsize/pmap_bootstrap). Therefore, I think we can just use PCPU_SET(foo, bar) instead of __pcpu[0].foo = bar in pmap_bootstrap. With that, we can remove both the extern declration of __pcpu and the direct references to it from pmap.c.

I am fine with this. Another option is to provide machine/_pcpu.h where post-struct pcpu declarations may be placed. The header would be included by sys/pcpu.h.

[rlibby]

Avoid referring directly to the pcpu symbol from amd64/pmap.c, instead accessing it via PCPU_SET macros. Also delete an unused extern decl of pcpu from mp_x86.c.

[markj]

Approved