* [PATCHv2 0/2] Fix another VM_BUG_ON_PAGE(PageTail(page)) on mbind(2) @ 2016-02-02 16:20 Kirill A. Shutemov 2016-02-02 16:21 ` [PATCHv2 1/2] mempolicy: do not try to queue pages from !vma_migratable() Kirill A. Shutemov 2016-02-02 16:21 ` [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning Kirill A. Shutemov 0 siblings, 2 replies; 6+ messages in thread From: Kirill A. Shutemov @ 2016-02-02 16:20 UTC (permalink / raw) To: Andrew Morton, Dmitry Vyukov Cc: Vlastimil Babka, David Rientjes, Naoya Horiguchi, Michal Hocko, linux-mm, linux-kernel, Kirill A. Shutemov Dmitry Vyukov reported yet another VM_BUG_ON_PAGE(PageTail(page)) bug from isolate_lru_page(). The fisrt patch fixes the bug by filter out non migratable VMAs in queue_pages_test_walk(). There's no point to queue pages from non-migratable VMA even for MPOL_MF_STRICT. The second patch replace VM_BUG_ON_PAGE() with WARN_RATELIMIT() in isolate_lru_page(). Most attempts to isolate tail pages are not fatal, as these pages usually are not on LRU and will not be isolated. v2: - address feedback from Michal Hocko; Kirill A. Shutemov (2): mempolicy: do not try to queue pages from !vma_migratable() mm: downgrade VM_BUG in isolate_lru_page() to warning mm/mempolicy.c | 14 +++++--------- mm/vmscan.c | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) -- 2.7.0 -- 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] 6+ messages in thread
* [PATCHv2 1/2] mempolicy: do not try to queue pages from !vma_migratable() 2016-02-02 16:20 [PATCHv2 0/2] Fix another VM_BUG_ON_PAGE(PageTail(page)) on mbind(2) Kirill A. Shutemov @ 2016-02-02 16:21 ` Kirill A. Shutemov 2016-02-02 16:21 ` [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning Kirill A. Shutemov 1 sibling, 0 replies; 6+ messages in thread From: Kirill A. Shutemov @ 2016-02-02 16:21 UTC (permalink / raw) To: Andrew Morton, Dmitry Vyukov Cc: Vlastimil Babka, David Rientjes, Naoya Horiguchi, Michal Hocko, linux-mm, linux-kernel, Kirill A. Shutemov Maybe I miss some point, but I don't see a reason why we try to queue pages from non migratable VMAs. This testcase steps on VM_BUG_ON_PAGE() in isolate_lru_page(): #include <fcntl.h> #include <unistd.h> #include <stdio.h> #include <sys/mman.h> #include <numaif.h> #define SIZE 0x2000 int foo; int main() { int fd; char *p; unsigned long mask = 2; fd = open("/dev/sg0", O_RDWR); p = mmap(NULL, SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); /* Faultin pages */ foo = p[0] + p[0x1000]; mbind(p, SIZE, MPOL_BIND, &mask, 4, MPOL_MF_MOVE | MPOL_MF_STRICT); return 0; } The only case when we can queue pages from such VMA is MPOL_MF_STRICT plus MPOL_MF_MOVE or MPOL_MF_MOVE_ALL for VMA which has pages on LRU, but gfp mask is not sutable for migaration (see mapping_gfp_mask() check in vma_migratable()). That's looks like a bug to me. Let's filter out non-migratable vma at start of queue_pages_test_walk() and go to queue_pages_pte_range() only if MPOL_MF_MOVE or MPOL_MF_MOVE_ALL flag is set. Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Reported-by: Dmitry Vyukov <dvyukov@google.com> --- mm/mempolicy.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 27d135408a22..4c4187c0e1de 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -548,8 +548,7 @@ retry: goto retry; } - if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) - migrate_page_add(page, qp->pagelist, flags); + migrate_page_add(page, qp->pagelist, flags); } pte_unmap_unlock(pte - 1, ptl); cond_resched(); @@ -625,7 +624,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end, unsigned long endvma = vma->vm_end; unsigned long flags = qp->flags; - if (vma->vm_flags & VM_PFNMAP) + if (!vma_migratable(vma)) return 1; if (endvma > end) @@ -644,16 +643,13 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end, if (flags & MPOL_MF_LAZY) { /* Similar to task_numa_work, skip inaccessible VMAs */ - if (vma_migratable(vma) && - vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)) + if (vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)) change_prot_numa(vma, start, endvma); return 1; } - if ((flags & MPOL_MF_STRICT) || - ((flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) && - vma_migratable(vma))) - /* queue pages from current vma */ + /* queue pages from current vma */ + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) return 0; return 1; } -- 2.7.0 -- 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] 6+ messages in thread
* [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning 2016-02-02 16:20 [PATCHv2 0/2] Fix another VM_BUG_ON_PAGE(PageTail(page)) on mbind(2) Kirill A. Shutemov 2016-02-02 16:21 ` [PATCHv2 1/2] mempolicy: do not try to queue pages from !vma_migratable() Kirill A. Shutemov @ 2016-02-02 16:21 ` Kirill A. Shutemov 2016-02-02 20:58 ` Andrew Morton 2016-02-03 14:58 ` Michal Hocko 1 sibling, 2 replies; 6+ messages in thread From: Kirill A. Shutemov @ 2016-02-02 16:21 UTC (permalink / raw) To: Andrew Morton, Dmitry Vyukov Cc: Vlastimil Babka, David Rientjes, Naoya Horiguchi, Michal Hocko, linux-mm, linux-kernel, Kirill A. Shutemov Calling isolate_lru_page() is wrong and shouldn't happen, but it not nessesary fatal: the page just will not be isolated if it's not on LRU. Let's downgrade the VM_BUG_ON_PAGE() to WARN_RATELIMIT(). Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> --- mm/vmscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/vmscan.c b/mm/vmscan.c index eb3dd37ccd7c..71b1c29948db 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1443,7 +1443,7 @@ int isolate_lru_page(struct page *page) int ret = -EBUSY; VM_BUG_ON_PAGE(!page_count(page), page); - VM_BUG_ON_PAGE(PageTail(page), page); + WARN_RATELIMIT(PageTail(page), "trying to isolate tail page"); if (PageLRU(page)) { struct zone *zone = page_zone(page); -- 2.7.0 -- 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] 6+ messages in thread
* Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning 2016-02-02 16:21 ` [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning Kirill A. Shutemov @ 2016-02-02 20:58 ` Andrew Morton 2016-02-02 22:03 ` Kirill A. Shutemov 2016-02-03 14:58 ` Michal Hocko 1 sibling, 1 reply; 6+ messages in thread From: Andrew Morton @ 2016-02-02 20:58 UTC (permalink / raw) To: Kirill A. Shutemov Cc: Dmitry Vyukov, Vlastimil Babka, David Rientjes, Naoya Horiguchi, Michal Hocko, linux-mm, linux-kernel On Tue, 2 Feb 2016 19:21:01 +0300 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote: > Calling isolate_lru_page() is wrong and shouldn't happen, but it not > nessesary fatal: the page just will not be isolated if it's not on LRU. > > Let's downgrade the VM_BUG_ON_PAGE() to WARN_RATELIMIT(). > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > --- > mm/vmscan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index eb3dd37ccd7c..71b1c29948db 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1443,7 +1443,7 @@ int isolate_lru_page(struct page *page) > int ret = -EBUSY; > > VM_BUG_ON_PAGE(!page_count(page), page); > - VM_BUG_ON_PAGE(PageTail(page), page); > + WARN_RATELIMIT(PageTail(page), "trying to isolate tail page"); > > if (PageLRU(page)) { > struct zone *zone = page_zone(page); Confused. I thought mm-fix-bogus-vm_bug_on_page-in-isolate_lru_page.patch: --- a/mm/vmscan.c~mm-fix-bogus-vm_bug_on_page-in-isolate_lru_page +++ a/mm/vmscan.c @@ -1443,7 +1443,7 @@ int isolate_lru_page(struct page *page) int ret = -EBUSY; VM_BUG_ON_PAGE(!page_count(page), page); - VM_BUG_ON_PAGE(PageTail(page), page); + VM_BUG_ON_PAGE(PageLRU(page) && PageTail(page), page); if (PageLRU(page)) { struct zone *zone = page_zone(page); was better. We *know* that we sometimes encounter LRU pages here and we know that we handle them correctly. So why scare users by blurting out a warning about something for which we won't be taking any action? -- 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] 6+ messages in thread
* Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning 2016-02-02 20:58 ` Andrew Morton @ 2016-02-02 22:03 ` Kirill A. Shutemov 0 siblings, 0 replies; 6+ messages in thread From: Kirill A. Shutemov @ 2016-02-02 22:03 UTC (permalink / raw) To: Andrew Morton Cc: Kirill A. Shutemov, Dmitry Vyukov, Vlastimil Babka, David Rientjes, Naoya Horiguchi, Michal Hocko, linux-mm, linux-kernel On Tue, Feb 02, 2016 at 12:58:44PM -0800, Andrew Morton wrote: > On Tue, 2 Feb 2016 19:21:01 +0300 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote: > > > Calling isolate_lru_page() is wrong and shouldn't happen, but it not > > nessesary fatal: the page just will not be isolated if it's not on LRU. > > > > Let's downgrade the VM_BUG_ON_PAGE() to WARN_RATELIMIT(). > > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > --- > > mm/vmscan.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index eb3dd37ccd7c..71b1c29948db 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -1443,7 +1443,7 @@ int isolate_lru_page(struct page *page) > > int ret = -EBUSY; > > > > VM_BUG_ON_PAGE(!page_count(page), page); > > - VM_BUG_ON_PAGE(PageTail(page), page); > > + WARN_RATELIMIT(PageTail(page), "trying to isolate tail page"); > > > > if (PageLRU(page)) { > > struct zone *zone = page_zone(page); > > Confused. I thought mm-fix-bogus-vm_bug_on_page-in-isolate_lru_page.patch: > > --- a/mm/vmscan.c~mm-fix-bogus-vm_bug_on_page-in-isolate_lru_page > +++ a/mm/vmscan.c > @@ -1443,7 +1443,7 @@ int isolate_lru_page(struct page *page) > int ret = -EBUSY; > > VM_BUG_ON_PAGE(!page_count(page), page); > - VM_BUG_ON_PAGE(PageTail(page), page); > + VM_BUG_ON_PAGE(PageLRU(page) && PageTail(page), page); > > if (PageLRU(page)) { > struct zone *zone = page_zone(page); > > was better. We *know* that we sometimes encounter LRU pages here and > we know that we handle them correctly. So why scare users by blurting > out a warning about something for which we won't be taking any action? We will. If we try to isolate tail page something went wrong. It just shouldn't happen. Compound pages should be isolated by head page as only whole compound page is on LRU, not subpages. If we see tail page here it's most probably from broken driver which forgot to set VM_IO. With setting VM_IO on such VMA we would avoid useless scan through pte in them and save some time. Or maybe something else is broken. Like we forgot to split THP before migration. -- Kirill A. Shutemov -- 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] 6+ messages in thread
* Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning 2016-02-02 16:21 ` [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning Kirill A. Shutemov 2016-02-02 20:58 ` Andrew Morton @ 2016-02-03 14:58 ` Michal Hocko 1 sibling, 0 replies; 6+ messages in thread From: Michal Hocko @ 2016-02-03 14:58 UTC (permalink / raw) To: Kirill A. Shutemov Cc: Andrew Morton, Dmitry Vyukov, Vlastimil Babka, David Rientjes, Naoya Horiguchi, linux-mm, linux-kernel On Tue 02-02-16 19:21:01, Kirill A. Shutemov wrote: > Calling isolate_lru_page() is wrong and shouldn't happen, but it not > nessesary fatal: the page just will not be isolated if it's not on LRU. > > Let's downgrade the VM_BUG_ON_PAGE() to WARN_RATELIMIT(). This will trigger for !CONFIG_DEBUG_VM as well which I am not sure is necessary. I guess isolate_lru_page is not such a hot path so this would be acceptable. > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Anyway Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/vmscan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index eb3dd37ccd7c..71b1c29948db 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1443,7 +1443,7 @@ int isolate_lru_page(struct page *page) > int ret = -EBUSY; > > VM_BUG_ON_PAGE(!page_count(page), page); > - VM_BUG_ON_PAGE(PageTail(page), page); > + WARN_RATELIMIT(PageTail(page), "trying to isolate tail page"); > > if (PageLRU(page)) { > struct zone *zone = page_zone(page); > -- > 2.7.0 > -- 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] 6+ messages in thread
end of thread, other threads:[~2016-02-03 14:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-02 16:20 [PATCHv2 0/2] Fix another VM_BUG_ON_PAGE(PageTail(page)) on mbind(2) Kirill A. Shutemov 2016-02-02 16:21 ` [PATCHv2 1/2] mempolicy: do not try to queue pages from !vma_migratable() Kirill A. Shutemov 2016-02-02 16:21 ` [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning Kirill A. Shutemov 2016-02-02 20:58 ` Andrew Morton 2016-02-02 22:03 ` Kirill A. Shutemov 2016-02-03 14:58 ` Michal Hocko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).