Re: [PATCH 17/17] PM: hibernate: split and simplify hib_submit_io

Rafael J. Wysocki
2025-04-29T11:12:28Z

On Tue, Apr 22, 2025 at 4:27 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Split hib_submit_io into a sync and async version. The sync version is
> a small wrapper around bdev_rw_virt which implements all the logic to
> add a kernel direct mapping range to a bio and synchronously submits it,
> while the async version is slightly simplified using the
> bio_add_virt_nofail for adding the single range.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

LGTM, so

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

and please route it as needed along with the rest of the series.

> ---
> kernel/power/swap.c | 103 +++++++++++++++++++-------------------------
> 1 file changed, 45 insertions(+), 58 deletions(-)
>
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 80ff5f933a62..ad13c461b657 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -268,35 +268,26 @@ static void hib_end_io(struct bio *bio)
> bio_put(bio);
> }
>
> -static int hib_submit_io(blk_opf_t opf, pgoff_t page_off, void *addr,
> +static int hib_submit_io_sync(blk_opf_t opf, pgoff_t page_off, void *addr)
> +{
> + return bdev_rw_virt(file_bdev(hib_resume_bdev_file),
> + page_off * (PAGE_SIZE >> 9), addr, PAGE_SIZE, opf);
> +}
> +
> +static int hib_submit_io_async(blk_opf_t opf, pgoff_t page_off, void *addr,
> struct hib_bio_batch *hb)
> {
> - struct page *page = virt_to_page(addr);
> struct bio *bio;
> - int error = 0;
>
> bio = bio_alloc(file_bdev(hib_resume_bdev_file), 1, opf,
> GFP_NOIO | __GFP_HIGH);
> bio->bi_iter.bi_sector = page_off * (PAGE_SIZE >> 9);
> -
> - if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
> - pr_err("Adding page to bio failed at %llu\n",
> - (unsigned long long)bio->bi_iter.bi_sector);
> - bio_put(bio);
> - return -EFAULT;
> - }
> -
> - if (hb) {
> - bio->bi_end_io = hib_end_io;
> - bio->bi_private = hb;
> - atomic_inc(&hb->count);
> - submit_bio(bio);
> - } else {
> - error = submit_bio_wait(bio);
> - bio_put(bio);
> - }
> -
> - return error;
> + bio_add_virt_nofail(bio, addr, PAGE_SIZE);
> + bio->bi_end_io = hib_end_io;
> + bio->bi_private = hb;
> + atomic_inc(&hb->count);
> + submit_bio(bio);
> + return 0;
> }
>
> static int hib_wait_io(struct hib_bio_batch *hb)
> @@ -316,7 +307,7 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
> {
> int error;
>
> - hib_submit_io(REQ_OP_READ, swsusp_resume_block, swsusp_header, NULL);
> + hib_submit_io_sync(REQ_OP_READ, swsusp_resume_block, swsusp_header);
> if (!memcmp("SWAP-SPACE",swsusp_header->sig, 10) ||
> !memcmp("SWAPSPACE2",swsusp_header->sig, 10)) {
> memcpy(swsusp_header->orig_sig,swsusp_header->sig, 10);
> @@ -329,8 +320,8 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
> swsusp_header->flags = flags;
> if (flags & SF_CRC32_MODE)
> swsusp_header->crc32 = handle->crc32;
> - error = hib_submit_io(REQ_OP_WRITE | REQ_SYNC,
> - swsusp_resume_block, swsusp_header, NULL);
> + error = hib_submit_io_sync(REQ_OP_WRITE | REQ_SYNC,
> + swsusp_resume_block, swsusp_header);
> } else {
> pr_err("Swap header not found!\n");
> error = -ENODEV;
> @@ -380,36 +371,30 @@ static int swsusp_swap_check(void)
>
> static int write_page(void *buf, sector_t offset, struct hib_bio_batch *hb)
> {
> + gfp_t gfp = GFP_NOIO | __GFP_NOWARN | __GFP_NORETRY;
> void *src;
> int ret;
>
> if (!offset)
> return -ENOSPC;
>
> - if (hb) {
> - src = (void *)__get_free_page(GFP_NOIO | __GFP_NOWARN |
> - __GFP_NORETRY);
> - if (src) {
> - copy_page(src, buf);
> - } else {
> - ret = hib_wait_io(hb); /* Free pages */
> - if (ret)
> - return ret;
> - src = (void *)__get_free_page(GFP_NOIO |
> - __GFP_NOWARN |
> - __GFP_NORETRY);
> - if (src) {
> - copy_page(src, buf);
> - } else {
> - WARN_ON_ONCE(1);
> - hb = NULL; /* Go synchronous */
> - src = buf;
> - }
> - }
> - } else {
> - src = buf;
> + if (!hb)
> + goto sync_io;
> +
> + src = (void *)__get_free_page(gfp);
> + if (!src) {
> + ret = hib_wait_io(hb); /* Free pages */
> + if (ret)
> + return ret;
> + src = (void *)__get_free_page(gfp);
> + if (WARN_ON_ONCE(!src))
> + goto sync_io;
> }
> - return hib_submit_io(REQ_OP_WRITE | REQ_SYNC, offset, src, hb);
> +
> + copy_page(src, buf);
> + return hib_submit_io_async(REQ_OP_WRITE | REQ_SYNC, offset, src, hb);
> +sync_io:
> + return hib_submit_io_sync(REQ_OP_WRITE | REQ_SYNC, offset, buf);
> }
>
> static void release_swap_writer(struct swap_map_handle *handle)
> @@ -1041,7 +1026,7 @@ static int get_swap_reader(struct swap_map_handle *handle,
> return -ENOMEM;
> }
>
> - error = hib_submit_io(REQ_OP_READ, offset, tmp->map, NULL);
> + error = hib_submit_io_sync(REQ_OP_READ, offset, tmp->map);
> if (error) {
> release_swap_reader(handle);
> return error;
> @@ -1065,7 +1050,10 @@ static int swap_read_page(struct swap_map_handle *handle, void *buf,
> offset = handle->cur->entries[handle->k];
> if (!offset)
> return -EFAULT;
> - error = hib_submit_io(REQ_OP_READ, offset, buf, hb);
> + if (hb)
> + error = hib_submit_io_async(REQ_OP_READ, offset, buf, hb);
> + else
> + error = hib_submit_io_sync(REQ_OP_READ, offset, buf);
> if (error)
> return error;
> if (++handle->k >= MAP_PAGE_ENTRIES) {
> @@ -1590,8 +1578,8 @@ int swsusp_check(bool exclusive)
> BLK_OPEN_READ, holder, NULL);
> if (!IS_ERR(hib_resume_bdev_file)) {
> clear_page(swsusp_header);
> - error = hib_submit_io(REQ_OP_READ, swsusp_resume_block,
> - swsusp_header, NULL);
> + error = hib_submit_io_sync(REQ_OP_READ, swsusp_resume_block,
> + swsusp_header);
> if (error)
> goto put;
>
> @@ -1599,9 +1587,9 @@ int swsusp_check(bool exclusive)
> memcpy(swsusp_header->sig, swsusp_header->orig_sig, 10);
> swsusp_header_flags = swsusp_header->flags;
> /* Reset swap signature now */
> - error = hib_submit_io(REQ_OP_WRITE | REQ_SYNC,
> + error = hib_submit_io_sync(REQ_OP_WRITE | REQ_SYNC,
> swsusp_resume_block,
> - swsusp_header, NULL);
> + swsusp_header);
> } else {
> error = -EINVAL;
> }
> @@ -1650,13 +1638,12 @@ int swsusp_unmark(void)
> {
> int error;
>
> - hib_submit_io(REQ_OP_READ, swsusp_resume_block,
> - swsusp_header, NULL);
> + hib_submit_io_sync(REQ_OP_READ, swsusp_resume_block, swsusp_header);
> if (!memcmp(HIBERNATE_SIG,swsusp_header->sig, 10)) {
> memcpy(swsusp_header->sig,swsusp_header->orig_sig, 10);
> - error = hib_submit_io(REQ_OP_WRITE | REQ_SYNC,
> + error = hib_submit_io_sync(REQ_OP_WRITE | REQ_SYNC,
> swsusp_resume_block,
> - swsusp_header, NULL);
> + swsusp_header);
> } else {
> pr_err("Cannot find swsusp signature!\n");
> error = -ENODEV;
> --
> 2.47.2
>