Skip to contentSkip to conversation

hugetlb: Fix clear_user_highpage arguments

PATCH In review Ralf Baechle 2 participants 4 messages Sep 28, 2007

Subsystems (3)
Memory management MIPS HugeTLB
lore
Share & tools
lore

Ralf Baechle

author
#0 lore raw

PATCH 1/1 hugetlb: Fix clear_user_highpage arguments

The virtual address space argument of clear_user_highpage is supposed to be the virtual address where the page being cleared will eventually be mapped. This allows architectures with virtually indexed caches a few clever tricks. That sort of trick falls over in painful ways if the virtual address argument is wrong.

Signed-off-by Ralf Baechle <[email protected]>
mm/hugetlb.c +1 -1
index 84c795e..eab8c42 100644
@@ -42,7 +42,7 @@ static void clear_huge_page(struct page *page, unsigned long addr)
 	might_sleep();
 	for (i = 0; i < (HPAGE_SIZE/PAGE_SIZE); i++) {
 		cond_resched();
-		clear_user_highpage(page + i, addr);
+		clear_user_highpage(page + i, addr + i * PAGE_SIZE);
 	}
 }
maintainer
#1 lore raw
Quoted 5 lines · On Fri, 28 Sep 2007 17:35:45 +0100 Ralf Baechle <[email protected]> wrote:

The virtual address space argument of clear_user_highpage is supposed to be the virtual address where the page being cleared will eventually be mapped. This allows architectures with virtually indexed caches a few clever tricks. That sort of trick falls over in painful ways if the virtual address argument is wrong.

yeah, but only if you're using a weird CPU architecture ;)

Quoted 7 lines
Signed-off-by Ralf Baechle <[email protected]>
mm/hugetlb.c +0 -0
index 84c795e..eab8c42 100644

mm/hugetlb.c static void clear_huge_page(struct page *page, unsigned long addr) · line 48

Quoted 8 lines
mm/hugetlb.c +1 -1
@@ -42,7 +42,7 @@ static void clear_huge_page(struct page *page, unsigned long addr)
 	might_sleep();
 	for (i = 0; i < (HPAGE_SIZE/PAGE_SIZE); i++) {
 		cond_resched();
-		clear_user_highpage(page + i, addr);
+		clear_user_highpage(page + i, addr + i * PAGE_SIZE);
 	}
 }

I'll add this to the 2.6.23 queue. Is it needed in 2.6.22.x?

author
#2 lore raw
Quoted 8 lines · Andrew Morton
Quoted 6 lines

The virtual address space argument of clear_user_highpage is supposed to be the virtual address where the page being cleared will eventually be mapped. This allows architectures with virtually indexed caches a few clever tricks. That sort of trick falls over in painful ways if the virtual address argument is wrong.

yeah, but only if you're using a weird CPU architecture ;)

I guess once I convinced your employer that weird CPU architectures deliver more punch for the watt they stop being so weird ;-)

Quoted 16 lines
Quoted 6 lines
mm/hugetlb.c +0 -0
index 84c795e..eab8c42 100644

mm/hugetlb.c static void clear_huge_page(struct page *page, unsigned long addr) · line 48

Quoted 8 lines
mm/hugetlb.c +1 -1
@@ -42,7 +42,7 @@ static void clear_huge_page(struct page *page, unsigned long addr)
 	might_sleep();
 	for (i = 0; i < (HPAGE_SIZE/PAGE_SIZE); i++) {
 		cond_resched();
-		clear_user_highpage(page + i, addr);
+		clear_user_highpage(page + i, addr + i * PAGE_SIZE);
 	}
 }

I'll add this to the 2.6.23 queue. Is it needed in 2.6.22.x?

It's totally theoretical atm, MIPS doesn't support hugetlb and I'm not even working on it. I just happened to spot the issue.

Ralf

maintainer
#3 lore raw
Quoted 9 lines · On Fri, 28 Sep 2007 19:53:35 +0100 Ralf Baechle <[email protected]> wrote:
Quoted 7 lines · Andrew Morton
Quoted 6 lines

The virtual address space argument of clear_user_highpage is supposed to be the virtual address where the page being cleared will eventually be mapped. This allows architectures with virtually indexed caches a few clever tricks. That sort of trick falls over in painful ways if the virtual address argument is wrong.

Quoted 4 lines
Quoted 1 line

yeah, but only if you're using a weird CPU architecture ;)

I guess once I convinced your employer that weird CPU architectures deliver more punch for the watt they stop being so weird ;-)

<wonders what you've gone and done this time>

Quoted 17 lines
Quoted 16 lines
Quoted 6 lines
mm/hugetlb.c +0 -0
index 84c795e..eab8c42 100644

mm/hugetlb.c static void clear_huge_page(struct page *page, unsigned long addr) · line 48

Quoted 8 lines
mm/hugetlb.c +1 -1
@@ -42,7 +42,7 @@ static void clear_huge_page(struct page *page, unsigned long addr)
 	might_sleep();
 	for (i = 0; i < (HPAGE_SIZE/PAGE_SIZE); i++) {
 		cond_resched();
-		clear_user_highpage(page + i, addr);
+		clear_user_highpage(page + i, addr + i * PAGE_SIZE);
 	}
 }

I'll add this to the 2.6.23 queue. Is it needed in 2.6.22.x?

Quoted 2 lines

It's totally theoretical atm, MIPS doesn't support hugetlb and I'm not even working on it. I just happened to spot the issue.

sparc64 might care about this bug.

Anyway, I'll plop it in 2.6.23.