Re: [PATCH] powerpc/mm: Refactor the floor/ceiling check in hugetlb range freeing functions
On Fri, 6 Nov 2020 13:20:54 + (UTC), Christophe Leroy wrote:
> All hugetlb range freeing functions have a verification like the following,
> which only differs by the mask used, depending on the page table level.
>
> start &= MASK;
> if (start < floor)
> return;
> if (ceiling) {
> ceiling &= MASK;
> if (! ceiling)
> return;
> }
> if (end - 1 > ceiling - 1)
> return;
>
> [...]
Applied to powerpc/next.
[1/1] powerpc/mm: Refactor the floor/ceiling check in hugetlb range freeing
functions
https://git.kernel.org/powerpc/c/7bfe54b5f16561bb703de6482f880614ada8dbf2
cheers
Re: [PATCH] powerpc/mm: Refactor the floor/ceiling check in hugetlb range freeing functions
On Fri, 2020-11-06 at 13:20 +, Christophe Leroy wrote:
> All hugetlb range freeing functions have a verification like the following,
> which only differs by the mask used, depending on the page table level.
>
> start &= MASK;
> if (start < floor)
> return;
> if (ceiling) {
> ceiling &= MASK;
> if (! ceiling)
> return;
> }
> if (end - 1 > ceiling - 1)
> return;
>
> Refactor that into a helper function which takes the mask as
> an argument, returning true when [start;end[ is not fully
> contained inside [floor;ceiling[
>
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/mm/hugetlbpage.c | 56 ---
> 1 file changed, 19 insertions(+), 37 deletions(-)
>
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 36c3800769fb..f8d8a4988e15 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -294,6 +294,21 @@ static void hugepd_free(struct mmu_gather *tlb, void
> *hugepte)
> static inline void hugepd_free(struct mmu_gather *tlb, void *hugepte) {}
> #endif
>
> +/* Return true when the entry to be freed maps more than the area being
> freed */
> +static bool range_is_outside_limits(unsigned long start, unsigned long end,
> + unsigned long floor, unsigned long ceiling,
> + unsigned long mask)
> +{
> + if ((start & mask) < floor)
> + return true;
> + if (ceiling) {
> + ceiling &= mask;
> + if (!ceiling)
> + return true;
> + }
> + return end - 1 > ceiling - 1;
> +}
> +
> static void free_hugepd_range(struct mmu_gather *tlb, hugepd_t *hpdp, int
> pdshift,
> unsigned long start, unsigned long end,
> unsigned long floor, unsigned long ceiling)
> @@ -309,15 +324,7 @@ static void free_hugepd_range(struct mmu_gather *tlb,
> hugepd_t *hpdp, int pdshif
> if (shift > pdshift)
> num_hugepd = 1 << (shift - pdshift);
>
> - start &= pdmask;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= pdmask;
> - if (! ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, pdmask))
> return;
>
> for (i = 0; i < num_hugepd; i++, hpdp++)
> @@ -334,18 +341,9 @@ static void hugetlb_free_pte_range(struct mmu_gather
> *tlb, pmd_t *pmd,
> unsigned long addr, unsigned long end,
> unsigned long floor, unsigned long ceiling)
> {
> - unsigned long start = addr;
> pgtable_t token = pmd_pgtable(*pmd);
>
> - start &= PMD_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PMD_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(addr, end, floor, ceiling, PMD_MASK))
> return;
>
> pmd_clear(pmd);
> @@ -395,15 +393,7 @@ static void hugetlb_free_pmd_range(struct mmu_gather
> *tlb, pud_t *pud,
> addr, next, floor, ceiling);
> } while (addr = next, addr != end);
>
> - start &= PUD_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PUD_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, PUD_MASK))
> return;
>
> pmd = pmd_offset(pud, start);
> @@ -446,15 +436,7 @@ static void hugetlb_free_pud_range(struct mmu_gather
> *tlb, p4d_t *p4d,
> }
> } while (addr = next, addr != end);
>
> - start &= PGDIR_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PGDIR_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, PGDIR_MASK))
> return;
>
> pud = pud_offset(p4d, start);
Well, "start" is still in use in hugetlb_free_pmd_range() and
hugetlb_free_pud_range() after range_is_outside_limits(), but after this patch,
"start" is not longer has the bitmask, i.e., "no &=".
Anyway, reverting this commit from today's linux-next fixed a crash on POWE9 NV.
# runltp -f hugetlb
[ 7703.114640][T58070] LTP: starting hugemmap05_1 (hugemmap05 -m)
[ 7703.157792][ C99] [ cut here ]
[ 7703.158279][ C99] kernel BUG at arch/powerpc/mm/book3s64/pgtable.c:387!
[ 7703.158306][ C99] Oops: Exception in kernel mode, sig: 5 [#1]
[
Re: [PATCH] powerpc/mm: Refactor the floor/ceiling check in hugetlb range freeing functions
Christophe Leroy writes:
> All hugetlb range freeing functions have a verification like the following,
> which only differs by the mask used, depending on the page table level.
>
> start &= MASK;
> if (start < floor)
> return;
> if (ceiling) {
> ceiling &= MASK;
> if (! ceiling)
> return;
> }
> if (end - 1 > ceiling - 1)
> return;
>
> Refactor that into a helper function which takes the mask as
> an argument, returning true when [start;end[ is not fully
> contained inside [floor;ceiling[
>
Reviewed-by: Aneesh Kumar K.V
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/mm/hugetlbpage.c | 56 ---
> 1 file changed, 19 insertions(+), 37 deletions(-)
>
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 36c3800769fb..f8d8a4988e15 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -294,6 +294,21 @@ static void hugepd_free(struct mmu_gather *tlb, void
> *hugepte)
> static inline void hugepd_free(struct mmu_gather *tlb, void *hugepte) {}
> #endif
>
> +/* Return true when the entry to be freed maps more than the area being
> freed */
> +static bool range_is_outside_limits(unsigned long start, unsigned long end,
> + unsigned long floor, unsigned long ceiling,
> + unsigned long mask)
> +{
> + if ((start & mask) < floor)
> + return true;
> + if (ceiling) {
> + ceiling &= mask;
> + if (!ceiling)
> + return true;
> + }
> + return end - 1 > ceiling - 1;
> +}
> +
> static void free_hugepd_range(struct mmu_gather *tlb, hugepd_t *hpdp, int
> pdshift,
> unsigned long start, unsigned long end,
> unsigned long floor, unsigned long ceiling)
> @@ -309,15 +324,7 @@ static void free_hugepd_range(struct mmu_gather *tlb,
> hugepd_t *hpdp, int pdshif
> if (shift > pdshift)
> num_hugepd = 1 << (shift - pdshift);
>
> - start &= pdmask;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= pdmask;
> - if (! ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, pdmask))
> return;
>
> for (i = 0; i < num_hugepd; i++, hpdp++)
> @@ -334,18 +341,9 @@ static void hugetlb_free_pte_range(struct mmu_gather
> *tlb, pmd_t *pmd,
> unsigned long addr, unsigned long end,
> unsigned long floor, unsigned long ceiling)
> {
> - unsigned long start = addr;
> pgtable_t token = pmd_pgtable(*pmd);
>
> - start &= PMD_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PMD_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(addr, end, floor, ceiling, PMD_MASK))
> return;
>
> pmd_clear(pmd);
> @@ -395,15 +393,7 @@ static void hugetlb_free_pmd_range(struct mmu_gather
> *tlb, pud_t *pud,
> addr, next, floor, ceiling);
> } while (addr = next, addr != end);
>
> - start &= PUD_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PUD_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, PUD_MASK))
> return;
>
> pmd = pmd_offset(pud, start);
> @@ -446,15 +436,7 @@ static void hugetlb_free_pud_range(struct mmu_gather
> *tlb, p4d_t *p4d,
> }
> } while (addr = next, addr != end);
>
> - start &= PGDIR_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PGDIR_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, PGDIR_MASK))
> return;
>
> pud = pud_offset(p4d, start);
> --
> 2.25.0
[PATCH] powerpc/mm: Refactor the floor/ceiling check in hugetlb range freeing functions
All hugetlb range freeing functions have a verification like the following,
which only differs by the mask used, depending on the page table level.
start &= MASK;
if (start < floor)
return;
if (ceiling) {
ceiling &= MASK;
if (! ceiling)
return;
}
if (end - 1 > ceiling - 1)
return;
Refactor that into a helper function which takes the mask as
an argument, returning true when [start;end[ is not fully
contained inside [floor;ceiling[
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/hugetlbpage.c | 56 ---
1 file changed, 19 insertions(+), 37 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 36c3800769fb..f8d8a4988e15 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -294,6 +294,21 @@ static void hugepd_free(struct mmu_gather *tlb, void
*hugepte)
static inline void hugepd_free(struct mmu_gather *tlb, void *hugepte) {}
#endif
+/* Return true when the entry to be freed maps more than the area being freed
*/
+static bool range_is_outside_limits(unsigned long start, unsigned long end,
+ unsigned long floor, unsigned long ceiling,
+ unsigned long mask)
+{
+ if ((start & mask) < floor)
+ return true;
+ if (ceiling) {
+ ceiling &= mask;
+ if (!ceiling)
+ return true;
+ }
+ return end - 1 > ceiling - 1;
+}
+
static void free_hugepd_range(struct mmu_gather *tlb, hugepd_t *hpdp, int
pdshift,
unsigned long start, unsigned long end,
unsigned long floor, unsigned long ceiling)
@@ -309,15 +324,7 @@ static void free_hugepd_range(struct mmu_gather *tlb,
hugepd_t *hpdp, int pdshif
if (shift > pdshift)
num_hugepd = 1 << (shift - pdshift);
- start &= pdmask;
- if (start < floor)
- return;
- if (ceiling) {
- ceiling &= pdmask;
- if (! ceiling)
- return;
- }
- if (end - 1 > ceiling - 1)
+ if (range_is_outside_limits(start, end, floor, ceiling, pdmask))
return;
for (i = 0; i < num_hugepd; i++, hpdp++)
@@ -334,18 +341,9 @@ static void hugetlb_free_pte_range(struct mmu_gather *tlb,
pmd_t *pmd,
unsigned long addr, unsigned long end,
unsigned long floor, unsigned long ceiling)
{
- unsigned long start = addr;
pgtable_t token = pmd_pgtable(*pmd);
- start &= PMD_MASK;
- if (start < floor)
- return;
- if (ceiling) {
- ceiling &= PMD_MASK;
- if (!ceiling)
- return;
- }
- if (end - 1 > ceiling - 1)
+ if (range_is_outside_limits(addr, end, floor, ceiling, PMD_MASK))
return;
pmd_clear(pmd);
@@ -395,15 +393,7 @@ static void hugetlb_free_pmd_range(struct mmu_gather *tlb,
pud_t *pud,
addr, next, floor, ceiling);
} while (addr = next, addr != end);
- start &= PUD_MASK;
- if (start < floor)
- return;
- if (ceiling) {
- ceiling &= PUD_MASK;
- if (!ceiling)
- return;
- }
- if (end - 1 > ceiling - 1)
+ if (range_is_outside_limits(start, end, floor, ceiling, PUD_MASK))
return;
pmd = pmd_offset(pud, start);
@@ -446,15 +436,7 @@ static void hugetlb_free_pud_range(struct mmu_gather *tlb,
p4d_t *p4d,
}
} while (addr = next, addr != end);
- start &= PGDIR_MASK;
- if (start < floor)
- return;
- if (ceiling) {
- ceiling &= PGDIR_MASK;
- if (!ceiling)
- return;
- }
- if (end - 1 > ceiling - 1)
+ if (range_is_outside_limits(start, end, floor, ceiling, PGDIR_MASK))
return;
pud = pud_offset(p4d, start);
--
2.25.0
