[PATCH] mm: ratelimit end_swap_bio_write() error

[PATCH] mm: ratelimit end_swap_bio_write() error

[Sergey Senozhatsky]

Use the ratelimited printk() version for swap-device write error
reporting. We can use ZRAM as a swap-device, and the tricky part
here is that zsmalloc() stores compressed objects in memory, thus
it has to allocates pages during swap-out. If the system is short
on memory, then we begin to flood printk() log buffer with the
same "Write-error on swap-device XXX" error messages and sometimes
simply lockup the system.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 mm/page_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_io.c b/mm/page_io.c
index e93f1a4cacd7..422cd49bcba8 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -63,7 +63,7 @@ void end_swap_bio_write(struct bio *bio)
 		 * Also clear PG_reclaim to avoid rotate_reclaimable_page()
 		 */
 		set_page_dirty(page);
-		pr_alert("Write-error on swap-device (%u:%u:%llu)\n",
+		pr_alert_ratelimited("Write-error on swap-device (%u:%u:%llu)\n",
 			 MAJOR(bio_dev(bio)), MINOR(bio_dev(bio)),
 			 (unsigned long long)bio->bi_iter.bi_sector);
 		ClearPageReclaim(page);
-- 
2.15.1

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

Re: [PATCH] mm: ratelimit end_swap_bio_write() error

[Michal Hocko]

On Sat 06-01-18 13:34:07, Sergey Senozhatsky wrote:
> Use the ratelimited printk() version for swap-device write error
> reporting. We can use ZRAM as a swap-device, and the tricky part
> here is that zsmalloc() stores compressed objects in memory, thus
> it has to allocates pages during swap-out. If the system is short
> on memory, then we begin to flood printk() log buffer with the
> same "Write-error on swap-device XXX" error messages and sometimes
> simply lockup the system.

Should we print an error in such a situation at all? Write-error
certainly sounds scare and it suggests something went really wrong.
My understading is that zram failed swap-out is not critical and
therefore the error message is not really useful. Or what should an
admin do when seeing it?

> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  mm/page_io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_io.c b/mm/page_io.c
> index e93f1a4cacd7..422cd49bcba8 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -63,7 +63,7 @@ void end_swap_bio_write(struct bio *bio)
>  		 * Also clear PG_reclaim to avoid rotate_reclaimable_page()
>  		 */
>  		set_page_dirty(page);
> -		pr_alert("Write-error on swap-device (%u:%u:%llu)\n",
> +		pr_alert_ratelimited("Write-error on swap-device (%u:%u:%llu)\n",
>  			 MAJOR(bio_dev(bio)), MINOR(bio_dev(bio)),
>  			 (unsigned long long)bio->bi_iter.bi_sector);
>  		ClearPageReclaim(page);
> -- 
> 2.15.1

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

Re: [PATCH] mm: ratelimit end_swap_bio_write() error

[Sergey Senozhatsky]

Hello,

On (01/06/18 10:41), Michal Hocko wrote:
> On Sat 06-01-18 13:34:07, Sergey Senozhatsky wrote:
> > Use the ratelimited printk() version for swap-device write error
> > reporting. We can use ZRAM as a swap-device, and the tricky part
> > here is that zsmalloc() stores compressed objects in memory, thus
> > it has to allocates pages during swap-out. If the system is short
> > on memory, then we begin to flood printk() log buffer with the
> > same "Write-error on swap-device XXX" error messages and sometimes
> > simply lockup the system.
> 
> Should we print an error in such a situation at all? Write-error
> certainly sounds scare and it suggests something went really wrong.
> My understading is that zram failed swap-out is not critical and
> therefore the error message is not really useful.

I don't mind to get rid of it. up to you :)

> Or what should an admin do when seeing it?

zsmalloc allocation is just one possibility; an error in
compressing algorithm is another one, yet is rather unlikely.
most likely it's OOM which can cause problems. but in any case
it's sort of unclear what should be done. an error can be a
temporary one or a fatal one, just like in __swap_writepage()
case. so may be both write error printk()-s can be dropped.

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

Re: [PATCH] mm: ratelimit end_swap_bio_write() error

[Michal Hocko]

On Sat 06-01-18 19:03:13, Sergey Senozhatsky wrote:
> Hello,
> 
> On (01/06/18 10:41), Michal Hocko wrote:
> > On Sat 06-01-18 13:34:07, Sergey Senozhatsky wrote:
> > > Use the ratelimited printk() version for swap-device write error
> > > reporting. We can use ZRAM as a swap-device, and the tricky part
> > > here is that zsmalloc() stores compressed objects in memory, thus
> > > it has to allocates pages during swap-out. If the system is short
> > > on memory, then we begin to flood printk() log buffer with the
> > > same "Write-error on swap-device XXX" error messages and sometimes
> > > simply lockup the system.
> > 
> > Should we print an error in such a situation at all? Write-error
> > certainly sounds scare and it suggests something went really wrong.
> > My understading is that zram failed swap-out is not critical and
> > therefore the error message is not really useful.
> 
> I don't mind to get rid of it. up to you :)

I do not think we can get rid of it for all swap backends.

> > Or what should an admin do when seeing it?
> 
> zsmalloc allocation is just one possibility; an error in
> compressing algorithm is another one, yet is rather unlikely.
> most likely it's OOM which can cause problems. but in any case
> it's sort of unclear what should be done. an error can be a
> temporary one or a fatal one, just like in __swap_writepage()
> case. so may be both write error printk()-s can be dropped.

Then I would suggest starting with sorting out which of those errors are
critical and which are not and report the error accordingly. I am sorry
to be fuzzy here but I am not familiar with the code to be more
specific. Anyway ratelimiting sounds more like a paper over than a real
solution. Also it sounds quite scary that you can see so many failures
to actually lock up the system just by printing a message...
-- 
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>

Re: [PATCH] mm: ratelimit end_swap_bio_write() error

[Sergey Senozhatsky]

On (01/06/18 14:34), Michal Hocko wrote:
> > zsmalloc allocation is just one possibility; an error in
> > compressing algorithm is another one, yet is rather unlikely.
> > most likely it's OOM which can cause problems. but in any case
> > it's sort of unclear what should be done. an error can be a
> > temporary one or a fatal one, just like in __swap_writepage()
> > case. so may be both write error printk()-s can be dropped.
> 
> Then I would suggest starting with sorting out which of those errors are
> critical and which are not and report the error accordingly. I am sorry
> to be fuzzy here but I am not familiar with the code to be more
> specific. Anyway ratelimiting sounds more like a paper over than a real
> solution. Also it sounds quite scary that you can see so many failures
> to actually lock up the system just by printing a message...

the lockup is not the main problem and I'm not really trying to
address it here. we simply can fill up the entire kernel logbuf
with the same "Write-error on swap-device" errors.

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

Re: [PATCH] mm: ratelimit end_swap_bio_write() error

[Michal Hocko]

On Mon 08-01-18 10:58:18, Sergey Senozhatsky wrote:
> On (01/06/18 14:34), Michal Hocko wrote:
> > > zsmalloc allocation is just one possibility; an error in
> > > compressing algorithm is another one, yet is rather unlikely.
> > > most likely it's OOM which can cause problems. but in any case
> > > it's sort of unclear what should be done. an error can be a
> > > temporary one or a fatal one, just like in __swap_writepage()
> > > case. so may be both write error printk()-s can be dropped.
> > 
> > Then I would suggest starting with sorting out which of those errors are
> > critical and which are not and report the error accordingly. I am sorry
> > to be fuzzy here but I am not familiar with the code to be more
> > specific. Anyway ratelimiting sounds more like a paper over than a real
> > solution. Also it sounds quite scary that you can see so many failures
> > to actually lock up the system just by printing a message...
> 
> the lockup is not the main problem and I'm not really trying to
> address it here. we simply can fill up the entire kernel logbuf
> with the same "Write-error on swap-device" errors.

Your changelog is rather modest on the information. Could you be more
specific on how the problem actually happens, how likely it is?

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

Re: [PATCH] mm: ratelimit end_swap_bio_write() error

[Sergey Senozhatsky]

On (01/08/18 09:37), Michal Hocko wrote:
[..]
> > the lockup is not the main problem and I'm not really trying to
> > address it here. we simply can fill up the entire kernel logbuf
> > with the same "Write-error on swap-device" errors.
> 
> 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.

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

Re: [PATCH] mm: ratelimit end_swap_bio_write() error

[Sergey Senozhatsky]

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>

Re: [PATCH] mm: ratelimit end_swap_bio_write() error

[Michal Hocko]

On Fri 12-01-18 13:41:33, Sergey Senozhatsky wrote:
> 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?

I am sorry but I didn't get to look into this yet. I still _believe_
that the ratelimit is just papering over a real problem here. So I would
prefer if the real fix was done instead. Maybe that is not that easy
easy, I haven't checked. Maybe I just do not understand the issue here.
-- 
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>