-
Notifications
You must be signed in to change notification settings - Fork 379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
prov/opx: use page_sizes[OFI_PAGE_SIZE] instead of PAGE_SIZE #10308
base: main
Are you sure you want to change the base?
Conversation
7bd193b
to
5f08006
Compare
@charlesshereda Does this look ok to you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
I agree with @charlesshereda , c52e280 has a similar fix. |
@charlesshereda @shefty c52e280 fixes the issue for |
Can you use 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 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 |
OFI_PAGE_SIZE is an index, which is why you need to use OFI_PAGE_SIZE is not a portable name for PAGE_SIZE. |
5f08006
to
8ef492b
Compare
Thank you for the clarification, updated. |
This looks fine to Ben and me now. |
@Xeonacid Is this PR still needed? If yes, please update the commit title to match the code. |
Some arch like riscv does not have a standard defined PAGE_SIZE in sys/user.h. Signed-off-by: Zhuo Zhi <h.dwwwwww@gmail.com>
8ef492b
to
0129ead
Compare
Sure! Updated. |
Some arch like riscv does not have a standard defined PAGE_SIZE in sys/user.h.