Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1324245 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-02-02 17:30 +0100
    Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to  warning Andrew Morton <akpm@linux-foundation.org> - 2016-02-02 22:00 +0100
      Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to  warning "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-02-02 23:10 +0100
    Re: [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to  warning Michal Hocko <mhocko@kernel.org> - 2016-02-03 16:00 +0100

#1324245 — [PATCHv2 2/2] mm: downgrade VM_BUG in isolate_lru_page() to warning

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

[toc] | [next] | [standalone]

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

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?

[toc] | [prev] | [next] | [standalone]

#1324683 — 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@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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web