All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-08-13 13:55 ` Michal Hocko
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2012-08-13 13:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Hillf Danton, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
from vma) fixed pgoff calculation but it has replaced it by
vma_hugecache_offset which is not approapriate for offsets used for
vma_prio_tree_foreach because that one expects index in page units
rather than in huge_page_shift.
Using vma_hugecache_offset is not incorrect because the pgoff will fit
into the same vmas but it is confusing so the standard PAGE_SHIFT based
index calculation is used instead.

Cc: Hillf Danton <dhillf@gmail.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 mm/hugetlb.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c39e4be..a74ea31 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2462,7 +2462,8 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
 	 * from page cache lookup which is in HPAGE_SIZE units.
 	 */
 	address = address & huge_page_mask(h);
-	pgoff = vma_hugecache_offset(h, vma, address);
+	pgoff = ((address - vma->vm_start) >> PAGE_SHIFT) +
+			vma->vm_pgoff;
 	mapping = vma->vm_file->f_dentry->d_inode->i_mapping;
 
 	/*
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-08-13 13:55 ` Michal Hocko
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2012-08-13 13:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Hillf Danton, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
from vma) fixed pgoff calculation but it has replaced it by
vma_hugecache_offset which is not approapriate for offsets used for
vma_prio_tree_foreach because that one expects index in page units
rather than in huge_page_shift.
Using vma_hugecache_offset is not incorrect because the pgoff will fit
into the same vmas but it is confusing so the standard PAGE_SHIFT based
index calculation is used instead.

Cc: Hillf Danton <dhillf@gmail.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 mm/hugetlb.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c39e4be..a74ea31 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2462,7 +2462,8 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
 	 * from page cache lookup which is in HPAGE_SIZE units.
 	 */
 	address = address & huge_page_mask(h);
-	pgoff = vma_hugecache_offset(h, vma, address);
+	pgoff = ((address - vma->vm_start) >> PAGE_SHIFT) +
+			vma->vm_pgoff;
 	mapping = vma->vm_file->f_dentry->d_inode->i_mapping;
 
 	/*
-- 
1.7.10.4

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
  2012-08-13 13:55 ` Michal Hocko
@ 2012-08-16 12:45   ` Hillf Danton
  -1 siblings, 0 replies; 18+ messages in thread
From: Hillf Danton @ 2012-08-16 12:45 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, linux-kernel, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Mon, Aug 13, 2012 at 9:55 PM, Michal Hocko <mhocko@suse.cz> wrote:
> 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> from vma) fixed pgoff calculation but it has replaced it by
> vma_hugecache_offset which is not approapriate for offsets used for
> vma_prio_tree_foreach because that one expects index in page units
> rather than in huge_page_shift.
> Using vma_hugecache_offset is not incorrect because the pgoff will fit
> into the same vmas but it is confusing so the standard PAGE_SHIFT based
> index calculation is used instead.
>
> Cc: Hillf Danton <dhillf@gmail.com>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: David Rientjes <rientjes@google.com>
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> ---

Thanks

Acked-by: Hillf Danton <dhillf@gmail.com>


>  mm/hugetlb.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index c39e4be..a74ea31 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2462,7 +2462,8 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
>          * from page cache lookup which is in HPAGE_SIZE units.
>          */
>         address = address & huge_page_mask(h);
> -       pgoff = vma_hugecache_offset(h, vma, address);
> +       pgoff = ((address - vma->vm_start) >> PAGE_SHIFT) +
> +                       vma->vm_pgoff;
>         mapping = vma->vm_file->f_dentry->d_inode->i_mapping;
>
>         /*
> --
> 1.7.10.4
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-08-16 12:45   ` Hillf Danton
  0 siblings, 0 replies; 18+ messages in thread
From: Hillf Danton @ 2012-08-16 12:45 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, linux-kernel, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Mon, Aug 13, 2012 at 9:55 PM, Michal Hocko <mhocko@suse.cz> wrote:
> 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> from vma) fixed pgoff calculation but it has replaced it by
> vma_hugecache_offset which is not approapriate for offsets used for
> vma_prio_tree_foreach because that one expects index in page units
> rather than in huge_page_shift.
> Using vma_hugecache_offset is not incorrect because the pgoff will fit
> into the same vmas but it is confusing so the standard PAGE_SHIFT based
> index calculation is used instead.
>
> Cc: Hillf Danton <dhillf@gmail.com>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: David Rientjes <rientjes@google.com>
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> ---

Thanks

Acked-by: Hillf Danton <dhillf@gmail.com>


>  mm/hugetlb.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index c39e4be..a74ea31 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2462,7 +2462,8 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
>          * from page cache lookup which is in HPAGE_SIZE units.
>          */
>         address = address & huge_page_mask(h);
> -       pgoff = vma_hugecache_offset(h, vma, address);
> +       pgoff = ((address - vma->vm_start) >> PAGE_SHIFT) +
> +                       vma->vm_pgoff;
>         mapping = vma->vm_file->f_dentry->d_inode->i_mapping;
>
>         /*
> --
> 1.7.10.4
>

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
  2012-08-16 12:45   ` Hillf Danton
@ 2012-08-16 15:11     ` Michal Hocko
  -1 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2012-08-16 15:11 UTC (permalink / raw)
  To: Hillf Danton
  Cc: Andrew Morton, linux-mm, linux-kernel, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Thu 16-08-12 20:45:15, Hillf Danton wrote:
> On Mon, Aug 13, 2012 at 9:55 PM, Michal Hocko <mhocko@suse.cz> wrote:
> > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> > from vma) fixed pgoff calculation but it has replaced it by
> > vma_hugecache_offset which is not approapriate for offsets used for
> > vma_prio_tree_foreach because that one expects index in page units
> > rather than in huge_page_shift.
> > Using vma_hugecache_offset is not incorrect because the pgoff will fit
> > into the same vmas but it is confusing so the standard PAGE_SHIFT based
> > index calculation is used instead.
> >
> > Cc: Hillf Danton <dhillf@gmail.com>
> > Cc: Mel Gorman <mel@csn.ul.ie>
> > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Cc: Andrea Arcangeli <aarcange@redhat.com>
> > Cc: David Rientjes <rientjes@google.com>
> > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> > ---
> 
> Thanks
> 
> Acked-by: Hillf Danton <dhillf@gmail.com>

Thanks Hillf!

> >  mm/hugetlb.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index c39e4be..a74ea31 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -2462,7 +2462,8 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
> >          * from page cache lookup which is in HPAGE_SIZE units.
> >          */
> >         address = address & huge_page_mask(h);
> > -       pgoff = vma_hugecache_offset(h, vma, address);
> > +       pgoff = ((address - vma->vm_start) >> PAGE_SHIFT) +
> > +                       vma->vm_pgoff;
> >         mapping = vma->vm_file->f_dentry->d_inode->i_mapping;
> >
> >         /*
> > --
> > 1.7.10.4
> >

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-08-16 15:11     ` Michal Hocko
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2012-08-16 15:11 UTC (permalink / raw)
  To: Hillf Danton
  Cc: Andrew Morton, linux-mm, linux-kernel, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Thu 16-08-12 20:45:15, Hillf Danton wrote:
> On Mon, Aug 13, 2012 at 9:55 PM, Michal Hocko <mhocko@suse.cz> wrote:
> > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> > from vma) fixed pgoff calculation but it has replaced it by
> > vma_hugecache_offset which is not approapriate for offsets used for
> > vma_prio_tree_foreach because that one expects index in page units
> > rather than in huge_page_shift.
> > Using vma_hugecache_offset is not incorrect because the pgoff will fit
> > into the same vmas but it is confusing so the standard PAGE_SHIFT based
> > index calculation is used instead.
> >
> > Cc: Hillf Danton <dhillf@gmail.com>
> > Cc: Mel Gorman <mel@csn.ul.ie>
> > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Cc: Andrea Arcangeli <aarcange@redhat.com>
> > Cc: David Rientjes <rientjes@google.com>
> > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> > ---
> 
> Thanks
> 
> Acked-by: Hillf Danton <dhillf@gmail.com>

Thanks Hillf!

> >  mm/hugetlb.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index c39e4be..a74ea31 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -2462,7 +2462,8 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
> >          * from page cache lookup which is in HPAGE_SIZE units.
> >          */
> >         address = address & huge_page_mask(h);
> > -       pgoff = vma_hugecache_offset(h, vma, address);
> > +       pgoff = ((address - vma->vm_start) >> PAGE_SHIFT) +
> > +                       vma->vm_pgoff;
> >         mapping = vma->vm_file->f_dentry->d_inode->i_mapping;
> >
> >         /*
> > --
> > 1.7.10.4
> >

-- 
Michal Hocko
SUSE Labs

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
  2012-08-13 13:55 ` Michal Hocko
@ 2012-09-26 20:56   ` Johannes Weiner
  -1 siblings, 0 replies; 18+ messages in thread
From: Johannes Weiner @ 2012-09-26 20:56 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, linux-kernel, Hillf Danton, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Mon, Aug 13, 2012 at 03:55:41PM +0200, Michal Hocko wrote:
> 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> from vma) fixed pgoff calculation but it has replaced it by
> vma_hugecache_offset which is not approapriate for offsets used for
> vma_prio_tree_foreach because that one expects index in page units
> rather than in huge_page_shift.
> Using vma_hugecache_offset is not incorrect because the pgoff will fit
> into the same vmas but it is confusing so the standard PAGE_SHIFT based
> index calculation is used instead.

I do think it's incorrect.  The resulting index may not be too big,
but it can be too small: assume hpage size of 2M and the address to
unmap to be 0x200000.  This is regular page index 512 and hpage index
1.  If you have a VMA that maps the file only starting at the second
huge page, that VMAs vm_pgoff will be 512 but you ask for offset 1 and
miss it even though it does map the page of interest.  hugetlb_cow()
will try to unmap, miss the vma, and retry the cow until the
allocation succeeds or the skipped vma(s) go away.

Unless I missed something, this should not be deferred as a cleanup.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-09-26 20:56   ` Johannes Weiner
  0 siblings, 0 replies; 18+ messages in thread
From: Johannes Weiner @ 2012-09-26 20:56 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, linux-kernel, Hillf Danton, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Mon, Aug 13, 2012 at 03:55:41PM +0200, Michal Hocko wrote:
> 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> from vma) fixed pgoff calculation but it has replaced it by
> vma_hugecache_offset which is not approapriate for offsets used for
> vma_prio_tree_foreach because that one expects index in page units
> rather than in huge_page_shift.
> Using vma_hugecache_offset is not incorrect because the pgoff will fit
> into the same vmas but it is confusing so the standard PAGE_SHIFT based
> index calculation is used instead.

I do think it's incorrect.  The resulting index may not be too big,
but it can be too small: assume hpage size of 2M and the address to
unmap to be 0x200000.  This is regular page index 512 and hpage index
1.  If you have a VMA that maps the file only starting at the second
huge page, that VMAs vm_pgoff will be 512 but you ask for offset 1 and
miss it even though it does map the page of interest.  hugetlb_cow()
will try to unmap, miss the vma, and retry the cow until the
allocation succeeds or the skipped vma(s) go away.

Unless I missed something, this should not be deferred as a cleanup.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
  2012-09-26 20:56   ` Johannes Weiner
@ 2012-10-01 16:22     ` Michal Hocko
  -1 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2012-10-01 16:22 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, linux-mm, linux-kernel, Hillf Danton, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Wed 26-09-12 16:56:17, Johannes Weiner wrote:
> On Mon, Aug 13, 2012 at 03:55:41PM +0200, Michal Hocko wrote:
> > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> > from vma) fixed pgoff calculation but it has replaced it by
> > vma_hugecache_offset which is not approapriate for offsets used for
> > vma_prio_tree_foreach because that one expects index in page units
> > rather than in huge_page_shift.
> > Using vma_hugecache_offset is not incorrect because the pgoff will fit
> > into the same vmas but it is confusing so the standard PAGE_SHIFT based
> > index calculation is used instead.
> 
> I do think it's incorrect.  The resulting index may not be too big,
> but it can be too small: assume hpage size of 2M and the address to
> unmap to be 0x200000.  This is regular page index 512 and hpage index
> 1.  If you have a VMA that maps the file only starting at the second
> huge page, that VMAs vm_pgoff will be 512 but you ask for offset 1 and
> miss it even though it does map the page of interest.  hugetlb_cow()
> will try to unmap, miss the vma, and retry the cow until the
> allocation succeeds or the skipped vma(s) go away.
> 
> Unless I missed something, this should not be deferred as a cleanup.

You are right and I have totally missed this because I focused on the
other boundary too much :/ This vma_hugecache_offset is really
confusing.
Andrew has already updated the changelog so we will not get even more
confusion into the Linus tree.
Thanks for spotting this Johannes!
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-10-01 16:22     ` Michal Hocko
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2012-10-01 16:22 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, linux-mm, linux-kernel, Hillf Danton, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Wed 26-09-12 16:56:17, Johannes Weiner wrote:
> On Mon, Aug 13, 2012 at 03:55:41PM +0200, Michal Hocko wrote:
> > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> > from vma) fixed pgoff calculation but it has replaced it by
> > vma_hugecache_offset which is not approapriate for offsets used for
> > vma_prio_tree_foreach because that one expects index in page units
> > rather than in huge_page_shift.
> > Using vma_hugecache_offset is not incorrect because the pgoff will fit
> > into the same vmas but it is confusing so the standard PAGE_SHIFT based
> > index calculation is used instead.
> 
> I do think it's incorrect.  The resulting index may not be too big,
> but it can be too small: assume hpage size of 2M and the address to
> unmap to be 0x200000.  This is regular page index 512 and hpage index
> 1.  If you have a VMA that maps the file only starting at the second
> huge page, that VMAs vm_pgoff will be 512 but you ask for offset 1 and
> miss it even though it does map the page of interest.  hugetlb_cow()
> will try to unmap, miss the vma, and retry the cow until the
> allocation succeeds or the skipped vma(s) go away.
> 
> Unless I missed something, this should not be deferred as a cleanup.

You are right and I have totally missed this because I focused on the
other boundary too much :/ This vma_hugecache_offset is really
confusing.
Andrew has already updated the changelog so we will not get even more
confusion into the Linus tree.
Thanks for spotting this Johannes!
-- 
Michal Hocko
SUSE Labs

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
  2012-10-01 16:22     ` Michal Hocko
@ 2012-10-01 18:29       ` Johannes Weiner
  -1 siblings, 0 replies; 18+ messages in thread
From: Johannes Weiner @ 2012-10-01 18:29 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, linux-kernel, Hillf Danton, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Mon, Oct 01, 2012 at 06:22:26PM +0200, Michal Hocko wrote:
> On Wed 26-09-12 16:56:17, Johannes Weiner wrote:
> > On Mon, Aug 13, 2012 at 03:55:41PM +0200, Michal Hocko wrote:
> > > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> > > from vma) fixed pgoff calculation but it has replaced it by
> > > vma_hugecache_offset which is not approapriate for offsets used for
> > > vma_prio_tree_foreach because that one expects index in page units
> > > rather than in huge_page_shift.
> > > Using vma_hugecache_offset is not incorrect because the pgoff will fit
> > > into the same vmas but it is confusing so the standard PAGE_SHIFT based
> > > index calculation is used instead.
> > 
> > I do think it's incorrect.  The resulting index may not be too big,
> > but it can be too small: assume hpage size of 2M and the address to
> > unmap to be 0x200000.  This is regular page index 512 and hpage index
> > 1.  If you have a VMA that maps the file only starting at the second
> > huge page, that VMAs vm_pgoff will be 512 but you ask for offset 1 and
> > miss it even though it does map the page of interest.  hugetlb_cow()
> > will try to unmap, miss the vma, and retry the cow until the
> > allocation succeeds or the skipped vma(s) go away.
> > 
> > Unless I missed something, this should not be deferred as a cleanup.
> 
> You are right and I have totally missed this because I focused on the
> other boundary too much :/ This vma_hugecache_offset is really
> confusing.
> Andrew has already updated the changelog so we will not get even more
> confusion into the Linus tree.
> Thanks for spotting this Johannes!

Just saw the update on ozlabs, thanks guys.  Andrew, could you please
also add:

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-10-01 18:29       ` Johannes Weiner
  0 siblings, 0 replies; 18+ messages in thread
From: Johannes Weiner @ 2012-10-01 18:29 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, linux-kernel, Hillf Danton, Mel Gorman,
	KAMEZAWA Hiroyuki, Andrea Arcangeli, David Rientjes

On Mon, Oct 01, 2012 at 06:22:26PM +0200, Michal Hocko wrote:
> On Wed 26-09-12 16:56:17, Johannes Weiner wrote:
> > On Mon, Aug 13, 2012 at 03:55:41PM +0200, Michal Hocko wrote:
> > > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> > > from vma) fixed pgoff calculation but it has replaced it by
> > > vma_hugecache_offset which is not approapriate for offsets used for
> > > vma_prio_tree_foreach because that one expects index in page units
> > > rather than in huge_page_shift.
> > > Using vma_hugecache_offset is not incorrect because the pgoff will fit
> > > into the same vmas but it is confusing so the standard PAGE_SHIFT based
> > > index calculation is used instead.
> > 
> > I do think it's incorrect.  The resulting index may not be too big,
> > but it can be too small: assume hpage size of 2M and the address to
> > unmap to be 0x200000.  This is regular page index 512 and hpage index
> > 1.  If you have a VMA that maps the file only starting at the second
> > huge page, that VMAs vm_pgoff will be 512 but you ask for offset 1 and
> > miss it even though it does map the page of interest.  hugetlb_cow()
> > will try to unmap, miss the vma, and retry the cow until the
> > allocation succeeds or the skipped vma(s) go away.
> > 
> > Unless I missed something, this should not be deferred as a cleanup.
> 
> You are right and I have totally missed this because I focused on the
> other boundary too much :/ This vma_hugecache_offset is really
> confusing.
> Andrew has already updated the changelog so we will not get even more
> confusion into the Linus tree.
> Thanks for spotting this Johannes!

Just saw the update on ozlabs, thanks guys.  Andrew, could you please
also add:

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
  2012-08-13 13:49     ` Michal Hocko
@ 2012-08-13 13:51       ` Hillf Danton
  -1 siblings, 0 replies; 18+ messages in thread
From: Hillf Danton @ 2012-08-13 13:51 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Mel Gorman, Andrew Morton, Linux-MM, LKML, KAMEZAWA Hiroyuki,
	David Rientjes

On Mon, Aug 13, 2012 at 9:49 PM, Michal Hocko <mhocko@suse.cz> wrote:
>
> I will leave it as an excersise for the careful reader...

Is it too late for you to prepare a redelivery?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-08-13 13:51       ` Hillf Danton
  0 siblings, 0 replies; 18+ messages in thread
From: Hillf Danton @ 2012-08-13 13:51 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Mel Gorman, Andrew Morton, Linux-MM, LKML, KAMEZAWA Hiroyuki,
	David Rientjes

On Mon, Aug 13, 2012 at 9:49 PM, Michal Hocko <mhocko@suse.cz> wrote:
>
> I will leave it as an excersise for the careful reader...

Is it too late for you to prepare a redelivery?

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
  2012-08-13 13:24   ` Hillf Danton
@ 2012-08-13 13:49     ` Michal Hocko
  -1 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2012-08-13 13:49 UTC (permalink / raw)
  To: Hillf Danton
  Cc: Mel Gorman, Andrew Morton, Linux-MM, LKML, KAMEZAWA Hiroyuki,
	David Rientjes

On Mon 13-08-12 21:24:36, Hillf Danton wrote:
> On Mon, Aug 13, 2012 at 9:09 PM, Michal Hocko <mhocko@suse.cz> wrote:
> > On Mon 13-08-12 20:10:41, Hillf Danton wrote:
> >> On Sun, Aug 12, 2012 at 5:31 PM, Michal Hocko <mhocko@suse.cz> wrote:
> >> > From d07b88a70ee1dbcc96502c48cde878931e7deb38 Mon Sep 17 00:00:00 2001
> >> > From: Michal Hocko <mhocko@suse.cz>
> >> > Date: Fri, 10 Aug 2012 15:03:07 +0200
> >> > Subject: [PATCH] hugetlb: do not use vma_hugecache_offset for
> >> >  vma_prio_tree_foreach
> >> >
> >> > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> >> > from vma) fixed pgoff calculation but it has replaced it by
> >> > vma_hugecache_offset which is not approapriate for offsets used for
> >> > vma_prio_tree_foreach because that one expects index in page units
> >> > rather than in huge_page_shift.
> >>
> >>
> >> What if another case of vma_prio_tree_foreach in try_to_unmap_file
> >> is correct?
> >
> > That one is surely correct (linear_page_index converts the page offset).
> 
> But linear_page_index is not used in this patch, why?

I will leave it as an excersise for the careful reader...
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-08-13 13:49     ` Michal Hocko
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2012-08-13 13:49 UTC (permalink / raw)
  To: Hillf Danton
  Cc: Mel Gorman, Andrew Morton, Linux-MM, LKML, KAMEZAWA Hiroyuki,
	David Rientjes

On Mon 13-08-12 21:24:36, Hillf Danton wrote:
> On Mon, Aug 13, 2012 at 9:09 PM, Michal Hocko <mhocko@suse.cz> wrote:
> > On Mon 13-08-12 20:10:41, Hillf Danton wrote:
> >> On Sun, Aug 12, 2012 at 5:31 PM, Michal Hocko <mhocko@suse.cz> wrote:
> >> > From d07b88a70ee1dbcc96502c48cde878931e7deb38 Mon Sep 17 00:00:00 2001
> >> > From: Michal Hocko <mhocko@suse.cz>
> >> > Date: Fri, 10 Aug 2012 15:03:07 +0200
> >> > Subject: [PATCH] hugetlb: do not use vma_hugecache_offset for
> >> >  vma_prio_tree_foreach
> >> >
> >> > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> >> > from vma) fixed pgoff calculation but it has replaced it by
> >> > vma_hugecache_offset which is not approapriate for offsets used for
> >> > vma_prio_tree_foreach because that one expects index in page units
> >> > rather than in huge_page_shift.
> >>
> >>
> >> What if another case of vma_prio_tree_foreach in try_to_unmap_file
> >> is correct?
> >
> > That one is surely correct (linear_page_index converts the page offset).
> 
> But linear_page_index is not used in this patch, why?

I will leave it as an excersise for the careful reader...
-- 
Michal Hocko
SUSE Labs

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-08-13 13:24   ` Hillf Danton
  0 siblings, 0 replies; 18+ messages in thread
From: Hillf Danton @ 2012-08-13 13:24 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Mel Gorman, Andrew Morton, Linux-MM, LKML, KAMEZAWA Hiroyuki,
	David Rientjes

On Mon, Aug 13, 2012 at 9:09 PM, Michal Hocko <mhocko@suse.cz> wrote:
> On Mon 13-08-12 20:10:41, Hillf Danton wrote:
>> On Sun, Aug 12, 2012 at 5:31 PM, Michal Hocko <mhocko@suse.cz> wrote:
>> > From d07b88a70ee1dbcc96502c48cde878931e7deb38 Mon Sep 17 00:00:00 2001
>> > From: Michal Hocko <mhocko@suse.cz>
>> > Date: Fri, 10 Aug 2012 15:03:07 +0200
>> > Subject: [PATCH] hugetlb: do not use vma_hugecache_offset for
>> >  vma_prio_tree_foreach
>> >
>> > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
>> > from vma) fixed pgoff calculation but it has replaced it by
>> > vma_hugecache_offset which is not approapriate for offsets used for
>> > vma_prio_tree_foreach because that one expects index in page units
>> > rather than in huge_page_shift.
>>
>>
>> What if another case of vma_prio_tree_foreach in try_to_unmap_file
>> is correct?
>
> That one is surely correct (linear_page_index converts the page offset).

But linear_page_index is not used in this patch, why?

> Anyway do you actually have any _real_ objection to the patch?

I will sign ack only after I see your answers to my questions.
Feel free to info me if you are unlikely to answer questions, Michal.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
@ 2012-08-13 13:24   ` Hillf Danton
  0 siblings, 0 replies; 18+ messages in thread
From: Hillf Danton @ 2012-08-13 13:24 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Mel Gorman, Andrew Morton, Linux-MM, LKML, KAMEZAWA Hiroyuki,
	David Rientjes

On Mon, Aug 13, 2012 at 9:09 PM, Michal Hocko <mhocko@suse.cz> wrote:
> On Mon 13-08-12 20:10:41, Hillf Danton wrote:
>> On Sun, Aug 12, 2012 at 5:31 PM, Michal Hocko <mhocko@suse.cz> wrote:
>> > From d07b88a70ee1dbcc96502c48cde878931e7deb38 Mon Sep 17 00:00:00 2001
>> > From: Michal Hocko <mhocko@suse.cz>
>> > Date: Fri, 10 Aug 2012 15:03:07 +0200
>> > Subject: [PATCH] hugetlb: do not use vma_hugecache_offset for
>> >  vma_prio_tree_foreach
>> >
>> > 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
>> > from vma) fixed pgoff calculation but it has replaced it by
>> > vma_hugecache_offset which is not approapriate for offsets used for
>> > vma_prio_tree_foreach because that one expects index in page units
>> > rather than in huge_page_shift.
>>
>>
>> What if another case of vma_prio_tree_foreach in try_to_unmap_file
>> is correct?
>
> That one is surely correct (linear_page_index converts the page offset).

But linear_page_index is not used in this patch, why?

> Anyway do you actually have any _real_ objection to the patch?

I will sign ack only after I see your answers to my questions.
Feel free to info me if you are unlikely to answer questions, Michal.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-10-01 18:29 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-13 13:55 [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach Michal Hocko
2012-08-13 13:55 ` Michal Hocko
2012-08-16 12:45 ` Hillf Danton
2012-08-16 12:45   ` Hillf Danton
2012-08-16 15:11   ` Michal Hocko
2012-08-16 15:11     ` Michal Hocko
2012-09-26 20:56 ` Johannes Weiner
2012-09-26 20:56   ` Johannes Weiner
2012-10-01 16:22   ` Michal Hocko
2012-10-01 16:22     ` Michal Hocko
2012-10-01 18:29     ` Johannes Weiner
2012-10-01 18:29       ` Johannes Weiner
  -- strict thread matches above, loose matches on Subject: below --
2012-08-13 13:09 [patch] hugetlb: correct page offset index for sharing pmd Michal Hocko
2012-08-13 13:24 ` [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach Hillf Danton
2012-08-13 13:24   ` Hillf Danton
2012-08-13 13:49   ` Michal Hocko
2012-08-13 13:49     ` Michal Hocko
2012-08-13 13:51     ` Hillf Danton
2012-08-13 13:51       ` Hillf Danton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.