Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning

[Kirill A. Shutemov]

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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > ---
> >  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