Fixups for hibernate issues reported by Askar Safin
Subsystems (2)
cover Fixups for hibernate issues reported by Askar Safin
Askar Safin reported some usability issues with hibernate messageing in some failure cases. This series fixes those issues as well as some style issues I noticed while touching the function.
Mario Limonciello (AMD) (3):
PM: hibernate: Emit an error when image writing fails
PM: hibernate: Use atomic64_t for compressed_size variable
PM: hibernate: Fix style issues in save_compressed_image()
kernel/power/swap.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
Signature · 1 line
2.43.0
PATCH 1/3 PM: hibernate: Emit an error when image writing fails
If image writing fails a return code is passed up to the caller but none of the callers log anything to the log and so the only record of it is the return code that userspace gets.
Adjust the logging so that the image size and speed of writing is only emitted on success and if there is an error it's saved the logs.
--- kernel/power/swap.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
kernel/power/swap.c +6 -4
@@ -877,11 +877,13 @@ static int save_compressed_image(struct swap_map_handle *handle, stop =ktime_get ();if (!ret) ret = err2; -if (!ret) +if (!ret) { +swsusp_show_speed (start, stop, nr_to_write,"Wrote" ); +pr_info ("Image size after compression: %d kbytes\n" , + (atomic_read (&compressed_size) /1024 ));pr_info ("Image saving done\n" ); -swsusp_show_speed (start, stop, nr_to_write,"Wrote" ); -pr_info ("Image size after compression: %d kbytes\n" , - (atomic_read (&compressed_size) /1024 )); + }else +pr_err ("Image saving failed: %d\n" , ret); out_clean:hib_finish_batch (&hb);
Signature · 1 line
2.43.0
Re: [PATCH 1/3] PM: hibernate: Emit an error when image writing fails
"Mario Limonciello (AMD)" <[email protected]>:
Quoted 3 lines
If image writing fails a return code is passed up to the caller but none of the callers log anything to the log and so the only record of it is the return code that userspace gets.
Thank you! This is bug fix, so, please, add to this particular commit:
Quoted 2 lines
Cc <[email protected]>Fixes a06c6f5d3cc9 ("PM: hibernate: Move to crypto APIs for LZO compression")
a06c6f5d3cc9 introduced that pr_info("Image size after compression...")
Signature · 1 line
Askar Safin
PATCH 2/3 PM: hibernate: Use atomic64_t for compressed_size variable
In reply to Mario Limonciello (AMD)compressed_size can overflow, showing nonsensical values. Change from atomic_t to atomic64_t to prevent overflow.
--- kernel/power/swap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
kernel/power/swap.c +5 -5
@@ -635,7 +635,7 @@ struct cmp_data { };/* Indicates the image size after compression */ -static atomic_t compressed_size =ATOMIC_INIT (0 ); +static atomic64_t compressed_size =ATOMIC_INIT (0 );/* * Compression function that runs in its own thread.
@@ -664,7 +664,7 @@ static int compress_threadfn(void *data) d->ret =crypto_acomp_compress (d->cr); d->cmp_len = d->cr->dlen; -atomic_set (&compressed_size,atomic_read (&compressed_size) + d->cmp_len); +atomic64_add (d->cmp_len, &compressed_size);atomic_set_release (&d->stop,1 );wake_up (&d->done); }
@@ -696,7 +696,7 @@ static int save_compressed_image(struct swap_map_handle *handle,hib_init_batch (&hb); -atomic_set (&compressed_size,0 ); +atomic64_set (&compressed_size,0 );/* * We'll limit the number of threads for compression to limit memory
@@ -879,8 +879,8 @@ static int save_compressed_image(struct swap_map_handle *handle, ret = err2;if (!ret) {swsusp_show_speed (start, stop, nr_to_write,"Wrote" ); -pr_info ("Image size after compression: %d kbytes\n" , - (atomic_read (&compressed_size) /1024 )); +pr_info ("Image size after compression: %lld kbytes\n" , + (atomic64_read (&compressed_size) /1024 ));pr_info ("Image saving done\n" ); }else pr_err ("Image saving failed: %d\n" , ret);
Signature · 1 line
2.43.0
Re: [PATCH 2/3] PM: hibernate: Use atomic64_t for compressed_size variable
"Mario Limonciello (AMD)" <[email protected]>:
Quoted 2 lines
compressed_sizecan overflow, showing nonsensical values. Change fromatomic_ttoatomic64_tto prevent overflow.
This is bug fix, too. Please, add:
Quoted 2 lines
Cc <[email protected]>Fixes a06c6f5d3cc9 ("PM: hibernate: Move to crypto APIs for LZO compression")
a06c6f5d3cc9 introduced variable "compressed_size".
(Yes, this commit responsible for both bugs.)
kernel/power/swap.c static int compress_threadfn(void *data) · line 667
Quoted 1 line
kernel/power/swap.c +0
-1-atomic_set (&compressed_size,atomic_read (&compressed_size) + d->cmp_len);
So, we read atomic variable, then add something to it, and then write? This looks like textbook parallel programming error. Very strange. Anyway, you fixed this, Mario! Thank you!
Signature · 1 line
Askar Safin
Re: [PATCH 2/3] PM: hibernate: Use atomic64_t for compressed_size variable
In reply to Mario Limonciello (AMD)Quoted 25 lines · Mario Limonciello (AMD)
compressed_sizecan overflow, showing nonsensical values. Change fromatomic_ttoatomic64_tto prevent overflow.Reported-by Askar Safin <[email protected]>Signed-off-by Mario Limonciello (AMD) <[email protected]>--- kernel/power/swap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)kernel/power/swap.c +1
-1@@ -635,7 +635,7 @@ struct cmp_data { };/* Indicates the image size after compression */ -static atomic_t compressed_size =ATOMIC_INIT (0 ); +static atomic64_t compressed_size =ATOMIC_INIT (0 );/* * Compression function that runs in its own thread.
kernel/power/swap.c static int compress_threadfn(void *data) · line 667
Quoted 4 lines
kernel/power/swap.c +0
-1@@ -664,7 +664,7 @@ static int compress_threadfn(void *data) d->ret =crypto_acomp_compress (d->cr); d->cmp_len = d->cr->dlen; -atomic_set (&compressed_size,atomic_read (&compressed_size) + d->cmp_len);
Hmm, how did I miss this?
Quoted 15 lines
kernel/power/swap.c +2
-1+atomic64_add (d->cmp_len, &compressed_size);atomic_set_release (&d->stop,1 );wake_up (&d->done); }@@ -696,7 +696,7 @@ static int save_compressed_image(struct swap_map_handle *handle,hib_init_batch (&hb); -atomic_set (&compressed_size,0 ); +atomic64_set (&compressed_size,0 );/* * We'll limit the number of threads for compression to limit memory @@ -879,8 +879,8 @@ static int save_compressed_image(struct swap_map_handle *handle, ret = err2;
kernel/power/swap.c static int save_compressed_image(struct swap_map_handle *handle,
Quoted 10 lines
kernel/power/swap.c +2
-3if (!ret) {swsusp_show_speed (start, stop, nr_to_write,"Wrote" ); -pr_info ("Image size after compression: %d kbytes\n" , - (atomic_read (&compressed_size) /1024 )); +pr_info ("Image size after compression: %lld kbytes\n" , + (atomic64_read (&compressed_size) /1024 ));pr_info ("Image saving done\n" ); }else pr_err ("Image saving failed: %d\n" , ret); --
Applied as 6.18-rc material along with the other patches in the series, with some changelog adjustments, added tags etc.
Thanks!
PATCH 3/3 PM: hibernate: Fix style issues in save_compressed_image()
In reply to Mario Limonciello (AMD)Fix the two issues caught by checkpatch:
Trailing statements should be on next line. Prefer 'unsigned int' to bare use of 'unsigned'
--- kernel/power/swap.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/power/swap.c +3 -2
@@ -689,7 +689,7 @@ static int save_compressed_image(struct swap_map_handle *handle,ktime_t start;ktime_t stop;size_t off; -unsigned thr, run_threads, nr_threads; +unsigned int thr, run_threads, nr_threads;unsigned char *page =NULL ;struct cmp_data *data =NULL ;struct crc_data *crc =NULL ;
@@ -901,7 +901,8 @@ static int save_compressed_image(struct swap_map_handle *handle, }vfree (data); } -if (page)free_page ((unsigned long )page); +if (page) +free_page ((unsigned long )page);return ret; }
Signature · 1 line
2.43.0
"Mario Limonciello (AMD)" <[email protected]>:
Quoted 3 lines
Askar Safin reported some usability issues with hibernate messageing in some failure cases. This series fixes those issues as well as some style issues I noticed while touching the function.
I tested in Qemu. Now everything works. Thank you!
Signature · 1 line
Askar Safin
Quoted 9 lines · Askar Safin
"Mario Limonciello (AMD)" <[email protected]>:
Quoted 3 lines
Askar Safin reported some usability issues with hibernate messageing in some failure cases. This series fixes those issues as well as some style issues I noticed while touching the function.
I tested in Qemu. Now everything works. Thank you!
Tested-by <[email protected]>
Thanks! I'll let Rafael pick up the extra tags you had for the patches if he agrees with them.