Remove more unused identifiers from r351198.

[markj]

Summary

Jeff pointed out that I missed these in r351742. Also sprinkle
__read_mostly, since these variables are accessed by PHYS_TO_VM_PAGE.
We could go further and make vm_page_array a const pointer on amd64,
i.e.,

#ifdef __amd64__
vm_page_t const vm_page_array = (vm_page_t)VM_MIN_KERNEL_ADDRESS;
#else
vm_page_t vm_page_array;
#endif

But I don't really like having the inconsistent definition. I will
commit the __read_mostly change separately.

[alc]

sys/amd64/include/vmparam.h

220 ↗

(On Diff #62048)

Where is this used?

[markj]

• Remove PMAP_PA_ADDRESS.

[markj]

sys/amd64/include/vmparam.h

220 ↗

(On Diff #62048)

My mistake, I added it when I tried making the vm_page_array pointer constant.

[alc]

sys/vm/vm_page.h

457

Why remove the comment?

[markj]

Restore lost comment.

[markj]

sys/vm/vm_page.h

457

It's another unintended change from when I was testing a conditionalized definition of vm_page_array.

[alc]

We could go further and make vm_page_array a const pointer on amd64, ...

If you start down this path, I would go a step further: Declare vm_page_array as an array, instead of a pointer, and define the symbol's value to VM_MIN_KERNEL_ADDRESS. This would eliminate the memory dereference to retrieve the start of the array.

[markj]

In D21642#472450, @alc wrote:

We could go further and make vm_page_array a const pointer on amd64, ...

If you start down this path, I would go a step further: Declare vm_page_array as an array, instead of a pointer, and define the symbol's value to VM_MIN_KERNEL_ADDRESS. This would eliminate the memory dereference to retrieve the start of the array.

Sorry, I don't quite see what you are proposing. My suggestion also eliminated the memory dereference, but I don't see how declaring vm_page_array as an array would help anything.

[alc]

In D21642#472480, @markj wrote:

In D21642#472450, @alc wrote:

We could go further and make vm_page_array a const pointer on amd64, ...

If you start down this path, I would go a step further: Declare vm_page_array as an array, instead of a pointer, and define the symbol's value to VM_MIN_KERNEL_ADDRESS. This would eliminate the memory dereference to retrieve the start of the array.

Sorry, I don't quite see what you are proposing. My suggestion also eliminated the memory dereference, but I don't see how declaring vm_page_array as an array would help anything.

I'm afraid not. Consider this simplified example.

const int *ptr = (int *)0x10000000;

int
func(void)
{

        return (*ptr);
}

Compiled with cc -O2 -S foo4.c to produce:

         .text
        .file   "foo4.c"
        .globl  func                    # -- Begin function func
        .p2align        4, 0x90
        .type   func,@function
func:                                   # @func
        .cfi_startproc
# %bb.0:                                # %entry
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset %rbp, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register %rbp
        movq    ptr(%rip), %rax
        movl    (%rax), %eax
        popq    %rbp
        .cfi_def_cfa %rsp, 8
        retq
.Lfunc_end0:
        .size   func, .Lfunc_end0-func
        .cfi_endproc
                                        # -- End function
        .type   ptr,@object             # @ptr
        .data
        .globl  ptr
        .p2align        3
ptr:
        .quad   268435456
        .size   ptr, 8


        .ident  "FreeBSD clang version 8.0.1 (tags/RELEASE_801/final 366581) (based on LLVM 8.0.1)"
        .section        ".note.GNU-stack","",@progbits
        .addrsig

[markj]

In D21642#472496, @alc wrote:

In D21642#472480, @markj wrote:

In D21642#472450, @alc wrote:

We could go further and make vm_page_array a const pointer on amd64, ...

If you start down this path, I would go a step further: Declare vm_page_array as an array, instead of a pointer, and define the symbol's value to VM_MIN_KERNEL_ADDRESS. This would eliminate the memory dereference to retrieve the start of the array.

Sorry, I don't quite see what you are proposing. My suggestion also eliminated the memory dereference, but I don't see how declaring vm_page_array as an array would help anything.

I'm afraid not. Consider this simplified example.

const int *ptr = (int *)0x10000000;
...

My proposal would be equivalent to int *const ptr = (int *)0x10000000, which yields

func:                                   # @func                                              
        .cfi_startproc                                                                         
# %bb.0:                                # %entry                  
        pushq   %rbp                           
        .cfi_def_cfa_offset 16
        .cfi_offset %rbp, -16                                                                  
        movq    %rsp, %rbp                                                                     
        .cfi_def_cfa_register %rbp                                                             
        movl    268435456, %eax                
        popq    %rbp                       
        .cfi_def_cfa %rsp, 8                                                                   
        retq                                                                                   
.Lfunc_end0:                                                                                   
        .size   func, .Lfunc_end0-func                                                                                                                                                        
        .cfi_endproc

[alc]

In D21642#472499, @markj wrote:

In D21642#472496, @alc wrote:

In D21642#472480, @markj wrote:

In D21642#472450, @alc wrote:

We could go further and make vm_page_array a const pointer on amd64, ...

If you start down this path, I would go a step further: Declare vm_page_array as an array, instead of a pointer, and define the symbol's value to VM_MIN_KERNEL_ADDRESS. This would eliminate the memory dereference to retrieve the start of the array.

Sorry, I don't quite see what you are proposing. My suggestion also eliminated the memory dereference, but I don't see how declaring vm_page_array as an array would help anything.

I'm afraid not. Consider this simplified example.

const int *ptr = (int *)0x10000000;
...

My proposal would be equivalent to int *const ptr = (int *)0x10000000, which yields

func:                                   # @func                                              
        .cfi_startproc                                                                         
# %bb.0:                                # %entry                  
        pushq   %rbp                           
        .cfi_def_cfa_offset 16
        .cfi_offset %rbp, -16                                                                  
        movq    %rsp, %rbp                                                                     
        .cfi_def_cfa_register %rbp                                                             
        movl    268435456, %eax                
        popq    %rbp                       
        .cfi_def_cfa %rsp, 8                                                                   
        retq                                                                                   
.Lfunc_end0:                                                                                   
        .size   func, .Lfunc_end0-func                                                                                                                                                        
        .cfi_endproc

Yes, I put the const in the wrong place. So, code within vm_page.c will avoid the dereference, but code outside vm_page.c that uses the declaration from vm_page.h will still perform the dereference. Declaring vm_page_array as an array would deal with both cases.

[markj]

In D21642#472503, @alc wrote:

Yes, I put the const in the wrong place. So, code within vm_page.c will avoid the dereference, but code outside vm_page.c that uses the declaration from vm_page.h will still perform the dereference. Declaring vm_page_array as an array would deal with both cases.

I'm still missing something. vm_page_array cannot be defined in vm_page.h, and if it is not defined there I do not see how the compiler can elide a memory dereference. I also do not see how an array symbol can be initialized to VM_MIN_KERNEL_ADDRESS.

[kib]

In D21642#472677, @markj wrote:

In D21642#472503, @alc wrote:

Yes, I put the const in the wrong place. So, code within vm_page.c will avoid the dereference, but code outside vm_page.c that uses the declaration from vm_page.h will still perform the dereference. Declaring vm_page_array as an array would deal with both cases.

I'm still missing something. vm_page_array cannot be defined in vm_page.h, and if it is not defined there I do not see how the compiler can elide a memory dereference.

It cannot be defined, but can be declared, same as now. Or I do not understand what you are trying to say.

I also do not see how an array symbol can be initialized to VM_MIN_KERNEL_ADDRESS.

You use .set in some asm file. No C definition is provided.

[alc]

In D21642#472706, @kib wrote:

In D21642#472677, @markj wrote:

In D21642#472503, @alc wrote:

Yes, I put the const in the wrong place. So, code within vm_page.c will avoid the dereference, but code outside vm_page.c that uses the declaration from vm_page.h will still perform the dereference. Declaring vm_page_array as an array would deal with both cases.

I'm still missing something. vm_page_array cannot be defined in vm_page.h, and if it is not defined there I do not see how the compiler can elide a memory dereference.

It cannot be defined, but can be declared, same as now. Or I do not understand what you are trying to say.

I also do not see how an array symbol can be initialized to VM_MIN_KERNEL_ADDRESS.

You use .set in some asm file. No C definition is provided.

In other words, the following. Please do note the comment at the bottom.

#define make_string(s) #s
#define value_to_string(v) make_string(v)

#define VM_MIN_KERNEL_ADDRESS   0x10000000

struct vm_page {
        int     dummy;
};

/*
 * Replace the declaration
 *
 *      extern vm_page_t vm_page_array;
 *
 * in vm/vm_page.h with the following on amd64:
 */
extern struct vm_page vm_page_array[];

/*
 * Then, on amd64, replace the definition in vm/vm_page.c with the following
 * in one .c or .s file:
 */
__asm (".set vm_page_array, " value_to_string(VM_MIN_KERNEL_ADDRESS));

int
main(void)
{

        return (vm_page_array[0].dummy);
}

/*
 * Which compiles (and links) to:
 *
00000000002012d0 <main>:
  2012d0:       55                      push   %rbp
  2012d1:       48 89 e5                mov    %rsp,%rbp
  2012d4:       8b 05 26 ed df 0f       mov    0xfdfed26(%rip),%eax        # 10000000 <vm_page_array>
  2012da:       5d                      pop    %rbp
  2012db:       c3                      retq
  2012dc:       cc                      int3
  2012dd:       cc                      int3
  2012de:       cc                      int3
  2012df:       cc                      int3
 *
 * The trouble is that vm_page_array won't be in the top 2GB of the address
 * space, so the compiler would have to be convinced to use a different
 * addressing mode.
 */

[markj]

Indeed, I cannot see a way to make the suggestion work without changing the code model for all files which reference vm_page_array. There is no "model" attribute for variables.