| Message ID | 20240925155540.4087150-1-lsahn@ooseel.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mmzone: remove redundant exception handler on pfn_valid() | expand |
On Thu, Sep 26, 2024 at 12:55:28AM +0900, Leesoo Ahn wrote: > 1). It can ensure the upper PAGE_SHIFT'ed bits of PHYS_PFN(..) are clear > by the C11 standard: I don't think you understand the purpose of the code you're removing. > +++ b/include/linux/mmzone.h > @@ -2028,15 +2028,6 @@ static inline int pfn_valid(unsigned long pfn) > struct mem_section *ms; > int ret; > > - /* > - * Ensure the upper PAGE_SHIFT bits are clear in the > - * pfn. Else it might lead to false positives when > - * some of the upper bits are set, but the lower bits > - * match a valid pfn. > - */ > - if (PHYS_PFN(PFN_PHYS(pfn)) != pfn) > - return 0; Let's assume that the pfn being asked about is 0x8000'0000'0000'1234. The actually valid PFNs are only 0-0x1000'0000. This function should return false. With your change, it will retrun true because it'll only check the bottom 52 bits, and ignore that some of the top 12 bits are set.
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index 17506e4a2835..164ae80977d6 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -2028,15 +2028,6 @@ static inline int pfn_valid(unsigned long pfn) struct mem_section *ms; int ret; - /* - * Ensure the upper PAGE_SHIFT bits are clear in the - * pfn. Else it might lead to false positives when - * some of the upper bits are set, but the lower bits - * match a valid pfn. - */ - if (PHYS_PFN(PFN_PHYS(pfn)) != pfn) - return 0; - if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS) return 0; ms = __pfn_to_section(pfn);
1). It can ensure the upper PAGE_SHIFT'ed bits of PHYS_PFN(..) are clear by the C11 standard: 6.5.7 Bitwise shift operator ... The result of E1 >> E2 is E1 right-shifted E2 bit positions. If E1 has an unsigned type or if E1 has a signed type and a nonnegative value, the value of the result is the integral part of the quotient of E1 / 2^E2. @pfn parameter of pfn_valid(..) is unsigned long and PHYS_PFN(..) makes the upper bits clear(zero) always with it. Otherwise, the result of E1 / 2^E2 doesn't make sense in the document. 2). x86 instructions The following x86 instructions are the part of the redundant exception handler: ... movabs $0xfffffffffffff,%rax and %rdi,%rax cmp %rax,%rdi je 22 xor %eax,%eax jmp __x86_return_thunk 22: push %rbp ... %rdi would be the @pfn parameter of pfn_valid(..) and -1 assigned to %rax. 'je' always takes place by %rdi AND and CMP ops with %rax. Therefore, the exception handler is unnecessary. Signed-off-by: Leesoo Ahn <lsahn@ooseel.net> --- include/linux/mmzone.h | 9 --------- 1 file changed, 9 deletions(-)