I've hit the "address >= vma->vm_end" check in do_page_add_anon_rmap()
just once. The stack showed khugepaged allocation trying to compact
pages: the call to page_add_anon_rmap() coming from remove_migration_pte().
That path holds anon_vma lock, but does not hold mmap_sem: it can
therefore race with a split_vma(), and in commit 5f70b962ccc2 "mmap:
avoid unnecessary anon_vma lock" we just took away the anon_vma lock
protection when adjusting vma->vm_end.
I don't think that particular BUG_ON ever caught anything interesting,
so better replace it by a comment, than reinstate the anon_vma locking.
Signed-off-by: Hugh Dickins <[email protected]>
---
mm/rmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- linux.orig/mm/rmap.c 2011-05-27 19:05:27.000000000 -0700
+++ linux/mm/rmap.c 2011-05-27 20:07:44.601361236 -0700
@@ -1014,7 +1014,7 @@ void do_page_add_anon_rmap(struct page *
return;
VM_BUG_ON(!PageLocked(page));
- VM_BUG_ON(address < vma->vm_start || address >= vma->vm_end);
+ /* address might be in next vma when migration races vma_adjust */
if (first)
__page_set_anon_rmap(page, vma, address, exclusive);
else
@@ -1709,7 +1709,7 @@ void hugepage_add_anon_rmap(struct page
BUG_ON(!PageLocked(page));
BUG_ON(!anon_vma);
- BUG_ON(address < vma->vm_start || address >= vma->vm_end);
+ /* address might be in next vma when migration races vma_adjust */
first = atomic_inc_and_test(&page->_mapcount);
if (first)
__hugepage_set_anon_rmap(page, vma, address, 0);
On Sunday 29 May 2011 04:17:04 Hugh Dickins wrote:
> I've hit the "address >= vma->vm_end" check in do_page_add_anon_rmap()
> just once. The stack showed khugepaged allocation trying to compact
> pages: the call to page_add_anon_rmap() coming from remove_migration_pte().
>
> That path holds anon_vma lock, but does not hold mmap_sem: it can
> therefore race with a split_vma(), and in commit 5f70b962ccc2 "mmap:
> avoid unnecessary anon_vma lock" we just took away the anon_vma lock
> protection when adjusting vma->vm_end.
>
> I don't think that particular BUG_ON ever caught anything interesting,
> so better replace it by a comment, than reinstate the anon_vma locking.
Is there another racing between "vma->vm_pgoff = pgoff;" in
vma_adjust() and linear_page_index() in __page_set_anon_rmap() ?
Nai Xia
>
> Signed-off-by: Hugh Dickins <[email protected]>
> ---
> mm/rmap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- linux.orig/mm/rmap.c 2011-05-27 19:05:27.000000000 -0700
> +++ linux/mm/rmap.c 2011-05-27 20:07:44.601361236 -0700
> @@ -1014,7 +1014,7 @@ void do_page_add_anon_rmap(struct page *
> return;
>
> VM_BUG_ON(!PageLocked(page));
> - VM_BUG_ON(address < vma->vm_start || address >= vma->vm_end);
> + /* address might be in next vma when migration races vma_adjust */
> if (first)
> __page_set_anon_rmap(page, vma, address, exclusive);
> else
> @@ -1709,7 +1709,7 @@ void hugepage_add_anon_rmap(struct page
>
> BUG_ON(!PageLocked(page));
> BUG_ON(!anon_vma);
> - BUG_ON(address < vma->vm_start || address >= vma->vm_end);
> + /* address might be in next vma when migration races vma_adjust */
> first = atomic_inc_and_test(&page->_mapcount);
> if (first)
> __hugepage_set_anon_rmap(page, vma, address, 0);
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. 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:"[email protected]"> [email protected] </a>
>
On Sunday 29 May 2011 18:23:46 Nai Xia wrote:
> On Sunday 29 May 2011 04:17:04 Hugh Dickins wrote:
> > I've hit the "address >= vma->vm_end" check in do_page_add_anon_rmap()
> > just once. The stack showed khugepaged allocation trying to compact
> > pages: the call to page_add_anon_rmap() coming from remove_migration_pte().
> >
> > That path holds anon_vma lock, but does not hold mmap_sem: it can
> > therefore race with a split_vma(), and in commit 5f70b962ccc2 "mmap:
> > avoid unnecessary anon_vma lock" we just took away the anon_vma lock
> > protection when adjusting vma->vm_end.
> >
> > I don't think that particular BUG_ON ever caught anything interesting,
> > so better replace it by a comment, than reinstate the anon_vma locking.
>
> Is there another racing between "vma->vm_pgoff = pgoff;" in
> vma_adjust() and linear_page_index() in __page_set_anon_rmap() ?
Oh, sorry, please ignore this, this _is_ protected by anon_vma lock.
Nai Xia
>
>
> Nai Xia
>
> >
> > Signed-off-by: Hugh Dickins <[email protected]>
> > ---
> > mm/rmap.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > --- linux.orig/mm/rmap.c 2011-05-27 19:05:27.000000000 -0700
> > +++ linux/mm/rmap.c 2011-05-27 20:07:44.601361236 -0700
> > @@ -1014,7 +1014,7 @@ void do_page_add_anon_rmap(struct page *
> > return;
> >
> > VM_BUG_ON(!PageLocked(page));
> > - VM_BUG_ON(address < vma->vm_start || address >= vma->vm_end);
> > + /* address might be in next vma when migration races vma_adjust */
> > if (first)
> > __page_set_anon_rmap(page, vma, address, exclusive);
> > else
> > @@ -1709,7 +1709,7 @@ void hugepage_add_anon_rmap(struct page
> >
> > BUG_ON(!PageLocked(page));
> > BUG_ON(!anon_vma);
> > - BUG_ON(address < vma->vm_start || address >= vma->vm_end);
> > + /* address might be in next vma when migration races vma_adjust */
> > first = atomic_inc_and_test(&page->_mapcount);
> > if (first)
> > __hugepage_set_anon_rmap(page, vma, address, 0);
> >
> > --
> > To unsubscribe, send a message with 'unsubscribe linux-mm' in
> > the body to [email protected]. 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:"[email protected]"> [email protected] </a>
> >
>