prov/opx: define PAGE_SIZE if undefined

[Xeonacid]

Some arch like riscv does not have a standard defined PAGE_SIZE in sys/user.h.

[j-xiong]

@charlesshereda Does this look ok to you?

[charlesshereda]

Requesting merge be withheld pending feedback from @j-xiong and why @Xeonacid felt existing code wasn't working given fix is already present.

[charlesshereda]

I think this should already be covered in c52e280, no? Plus that earlier commit protects it by specifically checking if it's a RISC-V arch.

[shefty]

I agree with @charlesshereda , c52e280 has a similar fix.

[Xeonacid]

@charlesshereda @shefty c52e280 fixes the issue for i_opx_hfi1_packet.h, but it is not included by opa_user_gen1.h.
Placing the fix in a header included by both headers might be a good idea. Do you have any suggestions for which header to choose?
Additionally, the fix in c52e280 is not entirely correct; it should be 4096UL instead of 4096 (int).

[shefty]

Can you use page_sizes[OFI_PAGE_SIZE], which gets the actual page size, rather than hard-coding 4k?

Looking at the existing code (well at least since my last update), it looks like PAGE_SIZE is only used to set OPX_HFI1_TID_PAGESIZE. Honestly, it's bad form for a provider to define a preprocessor define that's not protected using some sort of prefix. 'PAGE_SIZE' should probably be replaced with OPX_HFI1_TID_PAGESIZE. If PAGE_SIZE isn't defined, rather than defining it, define OPX_HFI1_TID_PAGESIZE to 4k or page_sizes[OFI_PAGE_SIZE] instead.

[Xeonacid]

Can you use page_sizes[OFI_PAGE_SIZE], which gets the actual page size, rather than hard-coding 4k?

Looking at the existing code (well at least since my last update), it looks like PAGE_SIZE is only used to set OPX_HFI1_TID_PAGESIZE. Honestly, it's bad form for a provider to define a preprocessor define that's not protected using some sort of prefix. 'PAGE_SIZE' should probably be replaced with OPX_HFI1_TID_PAGESIZE. If PAGE_SIZE isn't defined, rather than defining it, define OPX_HFI1_TID_PAGESIZE to 4k or page_sizes[OFI_PAGE_SIZE] instead.

I'm wondering how OFI_PAGE_SIZE should be used, which is an enum that should be 0:

enum {
	OFI_PAGE_SIZE,
	OFI_DEF_HUGEPAGE_SIZE,
};

Current code directly uses it as follows:

/* prov/psm3/shared/mem.c */
079 | page_sizes[OFI_PAGE_SIZE] = psize;

Replaces PAGE_SIZE with OFI_PAGE_SIZE should lead to a bad result.
Is there special handling for OFI_PAGE_SIZE in the code, or am I misunderstanding something?

[shefty]

OFI_PAGE_SIZE is an index, which is why you need to use page_sizes[OFI_PAGE_SIZE]. page_sizes contains an array of all page sizes supported by the system. The default page size entry is index 0, and the default huge page size is index 1.

OFI_PAGE_SIZE is not a portable name for PAGE_SIZE.

[Xeonacid]

OFI_PAGE_SIZE is an index, which is why you need to use page_sizes[OFI_PAGE_SIZE]. page_sizes contains an array of all page sizes supported by the system. The default page size entry is index 0, and the default huge page size is index 1.

OFI_PAGE_SIZE is not a portable name for PAGE_SIZE.

Thank you for the clarification, updated.

[charlesshereda]

This looks fine to Ben and me now.