* [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.