linux-kernel - Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 3 Feb 2016 00:03:23 +0200
From:	"Kirill A. Shutemov" <kirill@...temov.name>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	David Rientjes <rientjes@...gle.com>,
	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	Michal Hocko <mhocko@...e.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to
 warning

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@...ux.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@...ux.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