* [PATCH] PM: Batch hibernate and resume IO requests
@ 2020-09-22 16:19 Chen, Xiaoyi
2020-09-25 15:27 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Chen, Xiaoyi @ 2020-09-22 16:19 UTC (permalink / raw)
To: rjw, pavel, linux-pm; +Cc: Agarwal, Anchal, Duncan, David, dchinner, esandeen
Hibernate and resume process submits individual IO requests for each page
of the data. With this patch, we use blk_plug to improve the batching of
these requests.
Tested this patch with hibernate and resumes, and consistently observed the
merging of the IO requests. We see more than an order of magnitude
improvement in hibernate and resume speed.
One hibernate and resume cycle for 16GB used RAM out of 32GB takes around
21 minutes before the change, and 1 minutes after the change on systems
with limited storage IOPS.
Signed-off-by: Xiaoyi Chen <cxiaoyi@amazon.com>
Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
---
kernel/power/swap.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 01e2858b5fe3..961615365b5c 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -226,6 +226,7 @@ struct hib_bio_batch {
atomic_t count;
wait_queue_head_t wait;
blk_status_t error;
+ struct blk_plug plug;
};
static void hib_init_batch(struct hib_bio_batch *hb)
@@ -233,6 +234,12 @@ static void hib_init_batch(struct hib_bio_batch *hb)
atomic_set(&hb->count, 0);
init_waitqueue_head(&hb->wait);
hb->error = BLK_STS_OK;
+ blk_start_plug(&hb->plug);
+}
+
+static void hib_finish_batch(struct hib_bio_batch *hb)
+{
+ blk_finish_plug(&hb->plug);
}
static void hib_end_io(struct bio *bio)
@@ -294,6 +301,10 @@ static int hib_submit_io(int op, int op_flags, pgoff_t page_off, void *addr,
static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
{
+ /*
+ * We are relying on the behavior of blk_plug that a thread with
+ * a plug will flush the plug list before sleeping.
+ */
wait_event(hb->wait, atomic_read(&hb->count) == 0);
return blk_status_to_errno(hb->error);
}
@@ -561,6 +572,7 @@ static int save_image(struct swap_map_handle *handle,
nr_pages++;
}
err2 = hib_wait_io(&hb);
+ hib_finish_batch(&hb);
stop = ktime_get();
if (!ret)
ret = err2;
@@ -854,6 +866,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
pr_info("Image saving done\n");
swsusp_show_speed(start, stop, nr_to_write, "Wrote");
out_clean:
+ hib_finish_batch(&hb);
if (crc) {
if (crc->thr)
kthread_stop(crc->thr);
@@ -1084,6 +1097,7 @@ static int load_image(struct swap_map_handle *handle,
nr_pages++;
}
err2 = hib_wait_io(&hb);
+ hib_finish_batch(&hb);
stop = ktime_get();
if (!ret)
ret = err2;
@@ -1447,6 +1461,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
}
swsusp_show_speed(start, stop, nr_to_read, "Read");
out_clean:
+ hib_finish_batch(&hb);
for (i = 0; i < ring_size; i++)
free_page((unsigned long)page[i]);
if (crc) {
--
2.23.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] PM: Batch hibernate and resume IO requests
2020-09-22 16:19 [PATCH] PM: Batch hibernate and resume IO requests Chen, Xiaoyi
@ 2020-09-25 15:27 ` Rafael J. Wysocki
2020-09-25 19:26 ` Chen, Xiaoyi
0 siblings, 1 reply; 4+ messages in thread
From: Rafael J. Wysocki @ 2020-09-25 15:27 UTC (permalink / raw)
To: Chen, Xiaoyi
Cc: rjw, pavel, linux-pm, Agarwal, Anchal, Duncan, David, dchinner, esandeen
On Tue, Sep 22, 2020 at 6:19 PM Chen, Xiaoyi <cxiaoyi@amazon.com> wrote:
>
>
> Hibernate and resume process submits individual IO requests for each page
> of the data. With this patch, we use blk_plug to improve the batching of
> these requests.
>
> Tested this patch with hibernate and resumes, and consistently observed the
> merging of the IO requests. We see more than an order of magnitude
> improvement in hibernate and resume speed.
>
> One hibernate and resume cycle for 16GB used RAM out of 32GB takes around
> 21 minutes before the change, and 1 minutes after the change on systems
> with limited storage IOPS.
>
> Signed-off-by: Xiaoyi Chen <cxiaoyi@amazon.com>
> Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
> ---
> kernel/power/swap.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 01e2858b5fe3..961615365b5c 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -226,6 +226,7 @@ struct hib_bio_batch {
> atomic_t count;
> wait_queue_head_t wait;
> blk_status_t error;
> + struct blk_plug plug;
> };
>
> static void hib_init_batch(struct hib_bio_batch *hb)
> @@ -233,6 +234,12 @@ static void hib_init_batch(struct hib_bio_batch *hb)
> atomic_set(&hb->count, 0);
> init_waitqueue_head(&hb->wait);
> hb->error = BLK_STS_OK;
> + blk_start_plug(&hb->plug);
> +}
> +
> +static void hib_finish_batch(struct hib_bio_batch *hb)
> +{
> + blk_finish_plug(&hb->plug);
> }
>
> static void hib_end_io(struct bio *bio)
> @@ -294,6 +301,10 @@ static int hib_submit_io(int op, int op_flags, pgoff_t page_off, void *addr,
>
> static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
> {
> + /*
> + * We are relying on the behavior of blk_plug that a thread with
> + * a plug will flush the plug list before sleeping.
> + */
> wait_event(hb->wait, atomic_read(&hb->count) == 0);
> return blk_status_to_errno(hb->error);
> }
> @@ -561,6 +572,7 @@ static int save_image(struct swap_map_handle *handle,
> nr_pages++;
> }
> err2 = hib_wait_io(&hb);
> + hib_finish_batch(&hb);
> stop = ktime_get();
> if (!ret)
> ret = err2;
> @@ -854,6 +866,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
> pr_info("Image saving done\n");
> swsusp_show_speed(start, stop, nr_to_write, "Wrote");
> out_clean:
> + hib_finish_batch(&hb);
> if (crc) {
> if (crc->thr)
> kthread_stop(crc->thr);
> @@ -1084,6 +1097,7 @@ static int load_image(struct swap_map_handle *handle,
> nr_pages++;
> }
> err2 = hib_wait_io(&hb);
> + hib_finish_batch(&hb);
> stop = ktime_get();
> if (!ret)
> ret = err2;
> @@ -1447,6 +1461,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
> }
> swsusp_show_speed(start, stop, nr_to_read, "Read");
> out_clean:
> + hib_finish_batch(&hb);
> for (i = 0; i < ring_size; i++)
> free_page((unsigned long)page[i]);
> if (crc) {
> --
Applied as 5.10 material with some subject and changelog edits, but:
1. The patch is white-space-damaged and I needed to fix that up
manually which was not fun.
2. I dropped the second S-o-b, because it was not clear to me whether
a Co-developed-by tag was missing or Reviewed-by should have been used
instead.
Thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] PM: Batch hibernate and resume IO requests
2020-09-25 15:27 ` Rafael J. Wysocki
@ 2020-09-25 19:26 ` Chen, Xiaoyi
2020-09-28 13:59 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Chen, Xiaoyi @ 2020-09-25 19:26 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, linux-pm, Agarwal, Anchal, Duncan, David,
Dave Chinner, Eric Sandeen
On 9/25/20 4:27 PM, Rafael J. Wysocki wrote:
>
>
> On Tue, Sep 22, 2020 at 6:19 PM Chen, Xiaoyi <cxiaoyi@amazon.com> wrote:
>>
>>
>> Hibernate and resume process submits individual IO requests for each page
>> of the data. With this patch, we use blk_plug to improve the batching of
>> these requests.
>>
>> Tested this patch with hibernate and resumes, and consistently observed the
>> merging of the IO requests. We see more than an order of magnitude
>> improvement in hibernate and resume speed.
>>
>> One hibernate and resume cycle for 16GB used RAM out of 32GB takes around
>> 21 minutes before the change, and 1 minutes after the change on systems
>> with limited storage IOPS.
>>
>> Signed-off-by: Xiaoyi Chen <cxiaoyi@amazon.com>
>> Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
>> ---
>> kernel/power/swap.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
>> index 01e2858b5fe3..961615365b5c 100644
>> --- a/kernel/power/swap.c
>> +++ b/kernel/power/swap.c
>> @@ -226,6 +226,7 @@ struct hib_bio_batch {
>> atomic_t count;
>> wait_queue_head_t wait;
>> blk_status_t error;
>> + struct blk_plug plug;
>> };
>>
>> static void hib_init_batch(struct hib_bio_batch *hb)
>> @@ -233,6 +234,12 @@ static void hib_init_batch(struct hib_bio_batch *hb)
>> atomic_set(&hb->count, 0);
>> init_waitqueue_head(&hb->wait);
>> hb->error = BLK_STS_OK;
>> + blk_start_plug(&hb->plug);
>> +}
>> +
>> +static void hib_finish_batch(struct hib_bio_batch *hb)
>> +{
>> + blk_finish_plug(&hb->plug);
>> }
>>
>> static void hib_end_io(struct bio *bio)
>> @@ -294,6 +301,10 @@ static int hib_submit_io(int op, int op_flags, pgoff_t page_off, void *addr,
>>
>> static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
>> {
>> + /*
>> + * We are relying on the behavior of blk_plug that a thread with
>> + * a plug will flush the plug list before sleeping.
>> + */
>> wait_event(hb->wait, atomic_read(&hb->count) == 0);
>> return blk_status_to_errno(hb->error);
>> }
>> @@ -561,6 +572,7 @@ static int save_image(struct swap_map_handle *handle,
>> nr_pages++;
>> }
>> err2 = hib_wait_io(&hb);
>> + hib_finish_batch(&hb);
>> stop = ktime_get();
>> if (!ret)
>> ret = err2;
>> @@ -854,6 +866,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
>> pr_info("Image saving done\n");
>> swsusp_show_speed(start, stop, nr_to_write, "Wrote");
>> out_clean:
>> + hib_finish_batch(&hb);
>> if (crc) {
>> if (crc->thr)
>> kthread_stop(crc->thr);
>> @@ -1084,6 +1097,7 @@ static int load_image(struct swap_map_handle *handle,
>> nr_pages++;
>> }
>> err2 = hib_wait_io(&hb);
>> + hib_finish_batch(&hb);
>> stop = ktime_get();
>> if (!ret)
>> ret = err2;
>> @@ -1447,6 +1461,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
>> }
>> swsusp_show_speed(start, stop, nr_to_read, "Read");
>> out_clean:
>> + hib_finish_batch(&hb);
>> for (i = 0; i < ring_size; i++)
>> free_page((unsigned long)page[i]);
>> if (crc) {
>> --
>
> Applied as 5.10 material with some subject and changelog edits, but:
> 1. The patch is white-space-damaged and I needed to fix that up
> manually which was not fun.
> 2. I dropped the second S-o-b, because it was not clear to me whether
> a Co-developed-by tag was missing or Reviewed-by should have been used
> instead.
>
> Thanks!
>
Thanks for the prompt review. Apologies for the white-space and tags
issues, will do better next time.
Would you mind adding following tags if not too late?
Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
Co-Developed-by: Anchal Agarwal <anchalag@amazon.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] PM: Batch hibernate and resume IO requests
2020-09-25 19:26 ` Chen, Xiaoyi
@ 2020-09-28 13:59 ` Rafael J. Wysocki
0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2020-09-28 13:59 UTC (permalink / raw)
To: Chen, Xiaoyi
Cc: Rafael J. Wysocki, Pavel Machek, linux-pm, Agarwal, Anchal,
Duncan, David, Dave Chinner, Eric Sandeen
On Fri, Sep 25, 2020 at 9:26 PM Chen, Xiaoyi <cxiaoyi@amazon.com> wrote:
>
> On 9/25/20 4:27 PM, Rafael J. Wysocki wrote:
> >
> >
> > On Tue, Sep 22, 2020 at 6:19 PM Chen, Xiaoyi <cxiaoyi@amazon.com> wrote:
> >>
> >>
> >> Hibernate and resume process submits individual IO requests for each page
> >> of the data. With this patch, we use blk_plug to improve the batching of
> >> these requests.
> >>
> >> Tested this patch with hibernate and resumes, and consistently observed the
> >> merging of the IO requests. We see more than an order of magnitude
> >> improvement in hibernate and resume speed.
> >>
> >> One hibernate and resume cycle for 16GB used RAM out of 32GB takes around
> >> 21 minutes before the change, and 1 minutes after the change on systems
> >> with limited storage IOPS.
> >>
> >> Signed-off-by: Xiaoyi Chen <cxiaoyi@amazon.com>
> >> Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
> >> ---
> >> kernel/power/swap.c | 15 +++++++++++++++
> >> 1 file changed, 15 insertions(+)
> >>
> >> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> >> index 01e2858b5fe3..961615365b5c 100644
> >> --- a/kernel/power/swap.c
> >> +++ b/kernel/power/swap.c
> >> @@ -226,6 +226,7 @@ struct hib_bio_batch {
> >> atomic_t count;
> >> wait_queue_head_t wait;
> >> blk_status_t error;
> >> + struct blk_plug plug;
> >> };
> >>
> >> static void hib_init_batch(struct hib_bio_batch *hb)
> >> @@ -233,6 +234,12 @@ static void hib_init_batch(struct hib_bio_batch *hb)
> >> atomic_set(&hb->count, 0);
> >> init_waitqueue_head(&hb->wait);
> >> hb->error = BLK_STS_OK;
> >> + blk_start_plug(&hb->plug);
> >> +}
> >> +
> >> +static void hib_finish_batch(struct hib_bio_batch *hb)
> >> +{
> >> + blk_finish_plug(&hb->plug);
> >> }
> >>
> >> static void hib_end_io(struct bio *bio)
> >> @@ -294,6 +301,10 @@ static int hib_submit_io(int op, int op_flags, pgoff_t page_off, void *addr,
> >>
> >> static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
> >> {
> >> + /*
> >> + * We are relying on the behavior of blk_plug that a thread with
> >> + * a plug will flush the plug list before sleeping.
> >> + */
> >> wait_event(hb->wait, atomic_read(&hb->count) == 0);
> >> return blk_status_to_errno(hb->error);
> >> }
> >> @@ -561,6 +572,7 @@ static int save_image(struct swap_map_handle *handle,
> >> nr_pages++;
> >> }
> >> err2 = hib_wait_io(&hb);
> >> + hib_finish_batch(&hb);
> >> stop = ktime_get();
> >> if (!ret)
> >> ret = err2;
> >> @@ -854,6 +866,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
> >> pr_info("Image saving done\n");
> >> swsusp_show_speed(start, stop, nr_to_write, "Wrote");
> >> out_clean:
> >> + hib_finish_batch(&hb);
> >> if (crc) {
> >> if (crc->thr)
> >> kthread_stop(crc->thr);
> >> @@ -1084,6 +1097,7 @@ static int load_image(struct swap_map_handle *handle,
> >> nr_pages++;
> >> }
> >> err2 = hib_wait_io(&hb);
> >> + hib_finish_batch(&hb);
> >> stop = ktime_get();
> >> if (!ret)
> >> ret = err2;
> >> @@ -1447,6 +1461,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
> >> }
> >> swsusp_show_speed(start, stop, nr_to_read, "Read");
> >> out_clean:
> >> + hib_finish_batch(&hb);
> >> for (i = 0; i < ring_size; i++)
> >> free_page((unsigned long)page[i]);
> >> if (crc) {
> >> --
> >
> > Applied as 5.10 material with some subject and changelog edits, but:
> > 1. The patch is white-space-damaged and I needed to fix that up
> > manually which was not fun.
> > 2. I dropped the second S-o-b, because it was not clear to me whether
> > a Co-developed-by tag was missing or Reviewed-by should have been used
> > instead.
> >
> > Thanks!
> >
>
> Thanks for the prompt review. Apologies for the white-space and tags
> issues, will do better next time.
>
> Would you mind adding following tags if not too late?
> Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
> Co-Developed-by: Anchal Agarwal <anchalag@amazon.com>
Done, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-28 13:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-22 16:19 [PATCH] PM: Batch hibernate and resume IO requests Chen, Xiaoyi
2020-09-25 15:27 ` Rafael J. Wysocki
2020-09-25 19:26 ` Chen, Xiaoyi
2020-09-28 13:59 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).