Re: [PATCH] Fix virtual address handling in hugetlb fault

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Mon, 21 Nov 2011 14:27:20 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 21 Nov 2011 19:48:32 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > >From 7c29389be2890c6b6934a80b4841d07a7014fe26 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Mon, 21 Nov 2011 19:45:27 +0900
> > Subject: [PATCH] Fix virtual address handling in hugetlb fault
> > 
> > handle_mm_fault() passes 'faulted' address to hugetlb_fault().
> > Then, the address is not aligned to hugepage boundary.
> > 
> > Most of functions for hugetlb pages are aware of that and
> > calculate an alignment by itself. Some functions as copy_user_huge_page(),
> > and clear_huge_page() doesn't handle alignment by themselves.
> > 
> > This patch make hugeltb_fault() to calculate the alignment and pass
> > aligned addresss (top address of a faulted hugepage) to functions.
> > 
> 
> Does this actually fix any known user-visible misbehaviour?
> 

I just found this at reading codes. And I know 'vaddr' is ignored
in most of per-arch implemantation of clear_user_highpage().
It seems, in some arch, vaddr is used for flushing cache. Now,
CONFIG_HUGETLBFS can be set on x86,powerpc,ia64,mips,sh,sparc,tile. (by grep)

it seems mips and sh uses vaddr in clear_user_(high)page.



> It sounds like the code is masking addresses in a lot of different
> places.  It would be better to do it once, at the top level.  Perhaps
> this patch makes some of the existing masking obsolete?
> 

I think so. 
I'd like to check it and post an additional fix if this patch goes.


> > index bb28a5f..af37337 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -2629,6 +2629,8 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> >  	static DEFINE_MUTEX(hugetlb_instantiation_mutex);
> >  	struct hstate *h = hstate_vma(vma);
> >  
> > +	address = address & huge_page_mask(h);
> 
> --- a/mm/hugetlb.c~mm-hugetlbc-fix-virtual-address-handling-in-hugetlb-fault-fix
> +++ a/mm/hugetlb.c
> @@ -2639,7 +2639,7 @@ int hugetlb_fault(struct mm_struct *mm, 
>  	static DEFINE_MUTEX(hugetlb_instantiation_mutex);
>  	struct hstate *h = hstate_vma(vma);
>  
> -	address = address & huge_page_mask(h);
> +	address &= huge_page_mask(h);
>  
>  	ptep = huge_pte_offset(mm, address);
>  	if (ptep) {
> 
> is a bit more readable, IMO.
> 
Sure.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>