All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Minchan Kim <minchan@kernel.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [PATCH] mm: ratelimit end_swap_bio_write() error
Date: Fri, 12 Jan 2018 13:41:33 +0900	[thread overview]
Message-ID: <20180112044133.GA4314@jagdpanzerIV> (raw)
In-Reply-To: <20180108102234.GA818@jagdpanzerIV>

On (01/08/18 19:22), Sergey Senozhatsky wrote:
[..]
> > Your changelog is rather modest on the information.
> 
> fair point!
> 
> > Could you be more specific on how the problem actually happens how
> > likely it is?
> 
> ok. so what we have is
> 
> 	slow_path / swap-out page
> 	 __zram_bvec_write(page)
> 	  compressed_page = zcomp_compress(page)
> 	   zs_malloc(compressed_page)
> 	    // no available zspage found, need to allocate new
> 	     alloc_zspage()
> 	     {
> 		for (i = 0; i < class->pages_per_zspage; i++)
> 		    page = alloc_page(gfp);
> 		    if (!page)
> 			    return NULL
> 	     }
> 
> 	 return -ENOMEM
> 	...
> 	printk("Write-error on swap-device...");
> 
> 
> zspage-s can consist of up to ->pages_per_zspage normal pages.
> if alloc_page() fails then we can't allocate the entire zspage,
> so we can't store the swapped out page, so it remains in ram
> and we don't make any progress. so we try to swap another page
> and may be do the whole zs_malloc()->alloc_zspage() again, may
> be not. depending on how bad the OOM situation is there can be
> few or many "Write-error on swap-device" errors.
> 
> > And again, I do not think the throttling is an appropriate counter
> > measure. We do want to print those messages when a critical situation
> > happens. If we have a fallback then simply do not print at all.
> 
> sure, but with the ratelimited printk we still print those messages.
> we just don't print it for every single page we failed to write
> to the device. the existing error messages can (*sometimes*) be noisy
> and not very informative - "Write-error on swap-device (%u:%u:%llu)\n";
> it's not like 1000 of those tell more than 1 or 10.

Michal, does that make sense? with the updated/reworked commit
message will the patch be good enough?

	-ss

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

WARNING: multiple messages have this Message-ID (diff)
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Minchan Kim <minchan@kernel.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [PATCH] mm: ratelimit end_swap_bio_write() error
Date: Fri, 12 Jan 2018 13:41:33 +0900	[thread overview]
Message-ID: <20180112044133.GA4314@jagdpanzerIV> (raw)
In-Reply-To: <20180108102234.GA818@jagdpanzerIV>

On (01/08/18 19:22), Sergey Senozhatsky wrote:
[..]
> > Your changelog is rather modest on the information.
> 
> fair point!
> 
> > Could you be more specific on how the problem actually happens how
> > likely it is?
> 
> ok. so what we have is
> 
> 	slow_path / swap-out page
> 	 __zram_bvec_write(page)
> 	  compressed_page = zcomp_compress(page)
> 	   zs_malloc(compressed_page)
> 	    // no available zspage found, need to allocate new
> 	     alloc_zspage()
> 	     {
> 		for (i = 0; i < class->pages_per_zspage; i++)
> 		    page = alloc_page(gfp);
> 		    if (!page)
> 			    return NULL
> 	     }
> 
> 	 return -ENOMEM
> 	...
> 	printk("Write-error on swap-device...");
> 
> 
> zspage-s can consist of up to ->pages_per_zspage normal pages.
> if alloc_page() fails then we can't allocate the entire zspage,
> so we can't store the swapped out page, so it remains in ram
> and we don't make any progress. so we try to swap another page
> and may be do the whole zs_malloc()->alloc_zspage() again, may
> be not. depending on how bad the OOM situation is there can be
> few or many "Write-error on swap-device" errors.
> 
> > And again, I do not think the throttling is an appropriate counter
> > measure. We do want to print those messages when a critical situation
> > happens. If we have a fallback then simply do not print at all.
> 
> sure, but with the ratelimited printk we still print those messages.
> we just don't print it for every single page we failed to write
> to the device. the existing error messages can (*sometimes*) be noisy
> and not very informative - "Write-error on swap-device (%u:%u:%llu)\n";
> it's not like 1000 of those tell more than 1 or 10.

Michal, does that make sense? with the updated/reworked commit
message will the patch be good enough?

	-ss

  reply	other threads:[~2018-01-12  4:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-06  4:34 [PATCH] mm: ratelimit end_swap_bio_write() error Sergey Senozhatsky
2018-01-06  4:34 ` Sergey Senozhatsky
2018-01-06  9:41 ` Michal Hocko
2018-01-06  9:41   ` Michal Hocko
2018-01-06 10:03   ` Sergey Senozhatsky
2018-01-06 10:03     ` Sergey Senozhatsky
2018-01-06 13:34     ` Michal Hocko
2018-01-06 13:34       ` Michal Hocko
2018-01-08  1:58       ` Sergey Senozhatsky
2018-01-08  1:58         ` Sergey Senozhatsky
2018-01-08  8:37         ` Michal Hocko
2018-01-08  8:37           ` Michal Hocko
2018-01-08 10:22           ` Sergey Senozhatsky
2018-01-08 10:22             ` Sergey Senozhatsky
2018-01-12  4:41             ` Sergey Senozhatsky [this message]
2018-01-12  4:41               ` Sergey Senozhatsky
2018-01-12 12:27               ` Michal Hocko
2018-01-12 12:27                 ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180112044133.GA4314@jagdpanzerIV \
    --to=sergey.senozhatsky.work@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=minchan@kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=sergey.senozhatsky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.