Re: [PATCH 2/9] PM / Hibernate: add hibernate_io_ops

Rafael J. Wysocki
2010-06-24T15:25:19Z

On Wednesday, June 02, 2010, Jiri Slaby wrote:
> This is a centralized structure for actually used reader/writer. It
> contains basic operations called from snapshot layer which doesn't
> need to know who reads/writes the image right now -- this is
> specified by current set of functions stored in hibernate_io_ops.
>
> In other words there are more than one hibernate_io_ops which
> contain different set of functions. E.g. one is for swap, one for
> userspace (user.c) handler.

I don't like that user space reader/writer idea. I'll write more on that
commenting the next patches.

> For now they will hold only swap operations. In next patches, user
> support will be converted to ops as well to have a single layer which
> allows easier transitions later.

The patch looks good to me.

> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> ---
> kernel/power/hibernate.c | 2 +
> kernel/power/power.h | 27 +++++++++++++++++++++
> kernel/power/swap.c | 57 ++++++++++++++++++++++++++++++---------------
> 3 files changed, 67 insertions(+), 19 deletions(-)
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index aa9e916..7d38e6a 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -35,6 +35,8 @@ dev_t swsusp_resume_device;
> sector_t swsusp_resume_block;
> int in_suspend __nosavedata = 0;
>
> +struct hibernate_io_ops *hibernate_io_ops;
> +
> enum {
> HIBERNATION_INVALID,
> HIBERNATION_PLATFORM,
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 7427d54..32a40f9 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -139,6 +139,31 @@ static inline struct hibernate_io_handle *hib_io_handle_alloc(size_t priv_size)
> return ret;
> }
>
> +/**
> + * struct hibernate_io_ops - functions of a hibernation module
> + *
> + * @free_space: hom much space is available for the image

s/hom/how/

> + * @reader_start: initialize an image reader
> + * @reader_finish: deinitialize the reader
> + * @writer_start: initialize an image writer
> + * @writer_finish: deinitialize the writer
> + * @read_page: read an image page from a backing store
> + * @write_page: write an image page to a backing store
> + */
> +struct hibernate_io_ops {
> + unsigned long (*free_space)(void);
> +
> + struct hibernate_io_handle *(*reader_start)(unsigned int *flags_p);
> + int (*reader_finish)(struct hibernate_io_handle *io_handle);
> + struct hibernate_io_handle *(*writer_start)(void);
> + int (*writer_finish)(struct hibernate_io_handle *io_handle,
> + unsigned int flags, int error);
> + int (*read_page)(struct hibernate_io_handle *io_handle, void *addr,
> + struct bio **bio_chain);
> + int (*write_page)(struct hibernate_io_handle *io_handle, void *addr,
> + struct bio **bio_chain);
> +};
> +
> extern unsigned int snapshot_additional_pages(struct zone *zone);
> extern unsigned long snapshot_get_image_size(void);
> extern int snapshot_read_next(struct snapshot_handle *handle);
> @@ -237,6 +262,8 @@ enum {
>
> extern int pm_test_level;
>
> +extern struct hibernate_io_ops *hibernate_io_ops;
> +
> #ifdef CONFIG_SUSPEND_FREEZER
> static inline int suspend_freeze_processes(void)
> {
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 7096d20..f09494e 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -268,7 +268,7 @@ static void release_swap_writer(struct swap_map_handle *handle)
> handle->cur = NULL;
> }
>
> -static struct hibernate_io_handle *get_swap_writer(void)
> +static struct hibernate_io_handle *swap_writer_start(void)
> {
> struct hibernate_io_handle *io_handle;
> struct swap_map_handle *handle;
> @@ -399,7 +399,8 @@ static int save_image(struct hibernate_io_handle *io_handle,
> ret = snapshot_read_next(snapshot);
> if (ret <= 0)
> break;
> - ret = swap_write_page(io_handle, data_of(*snapshot), &bio);
> + ret = hibernate_io_ops->write_page(io_handle,
> + data_of(*snapshot), &bio);
> if (ret)
> break;
> if (!(nr_pages % m))
> @@ -419,18 +420,18 @@ static int save_image(struct hibernate_io_handle *io_handle,
> }
>
> /**
> - * enough_swap - Make sure we have enough swap to save the image.
> + * enough_space - Make sure we have enough space to save the image.
> *
> - * Returns TRUE or FALSE after checking the total amount of swap
> - * space avaiable from the resume partition.
> + * Returns TRUE or FALSE after checking the total amount of
> + * space avaiable from the resume block.
> */
>
> -static int enough_swap(unsigned int nr_pages)
> +static int enough_space(unsigned int nr_pages)
> {
> - unsigned int free_swap = count_swap_pages(root_swap, 1);
> + unsigned int free_pages = hibernate_io_ops->free_space();
>
> - pr_debug("PM: Free swap pages: %u\n", free_swap);
> - return free_swap > nr_pages + PAGES_FOR_IO;
> + pr_debug("PM: Free storage pages: %u\n", free_pages);
> + return free_pages > nr_pages + PAGES_FOR_IO;
> }
>
> /**
> @@ -452,13 +453,13 @@ int swsusp_write(unsigned int flags)
> int error;
>
> pages = snapshot_get_image_size();
> - io_handle = get_swap_writer();
> + io_handle = hibernate_io_ops->writer_start();
> if (IS_ERR(io_handle)) {
> printk(KERN_ERR "PM: Cannot get swap writer\n");
> return PTR_ERR(io_handle);
> }
> - if (!enough_swap(pages)) {
> - printk(KERN_ERR "PM: Not enough free swap\n");
> + if (!enough_space(pages)) {
> + printk(KERN_ERR "PM: Not enough free space for image\n");
> error = -ENOSPC;
> goto out_finish;
> }
> @@ -471,14 +472,19 @@ int swsusp_write(unsigned int flags)
> goto out_finish;
> }
> header = (struct swsusp_info *)data_of(snapshot);
> - error = swap_write_page(io_handle, header, NULL);
> + error = hibernate_io_ops->write_page(io_handle, header, NULL);
> if (!error)
> error = save_image(io_handle, &snapshot, pages - 1);
> out_finish:
> - error = swap_writer_finish(io_handle, flags, error);
> + error = hibernate_io_ops->writer_finish(io_handle, flags, error);
> return error;
> }
>
> +static unsigned long swap_free_space(void)
> +{
> + return count_swap_pages(root_swap, 1);
> +}
> +
> /**
> * The following functions allow us to read data using a swap map
> * in a file-alike way
> @@ -491,7 +497,7 @@ static void release_swap_reader(struct swap_map_handle *handle)
> handle->cur = NULL;
> }
>
> -static struct hibernate_io_handle *get_swap_reader(unsigned int *flags_p)
> +static struct hibernate_io_handle *swap_reader_start(unsigned int *flags_p)
> {
> struct hibernate_io_handle *io_handle;
> struct swap_map_handle *handle;
> @@ -561,6 +567,17 @@ static int swap_reader_finish(struct hibernate_io_handle *io_handle)
> return 0;
> }
>
> +struct hibernate_io_ops swap_ops = {
> + .free_space = swap_free_space,
> +
> + .reader_start = swap_reader_start,
> + .reader_finish = swap_reader_finish,
> + .writer_start = swap_writer_start,
> + .writer_finish = swap_writer_finish,
> + .read_page = swap_read_page,
> + .write_page = swap_write_page,
> +};
> +
> /**
> * load_image - load the image
> * @handle and the snapshot handle @snapshot
> @@ -591,7 +608,8 @@ static int load_image(struct hibernate_io_handle *io_handle,
> error = snapshot_write_next(snapshot);
> if (error <= 0)
> break;
> - error = swap_read_page(io_handle, data_of(*snapshot), &bio);
> + error = hibernate_io_ops->read_page(io_handle,
> + data_of(*snapshot), &bio);
> if (error)
> break;
> if (snapshot->sync_read)
> @@ -635,16 +653,16 @@ int swsusp_read(unsigned int *flags_p)
> if (error < PAGE_SIZE)
> return error < 0 ? error : -EFAULT;
> header = (struct swsusp_info *)data_of(snapshot);
> - io_handle = get_swap_reader(flags_p);
> + io_handle = hibernate_io_ops->reader_start(flags_p);
> if (IS_ERR(io_handle)) {
> error = PTR_ERR(io_handle);
> goto end;
> }
> if (!error)
> - error = swap_read_page(io_handle, header, NULL);
> + error = hibernate_io_ops->read_page(io_handle, header, NULL);
> if (!error)
> error = load_image(io_handle, &snapshot, header->pages - 1);
> - swap_reader_finish(io_handle);
> + hibernate_io_ops->reader_finish(io_handle);
> end:
> if (!error)
> pr_debug("PM: Image successfully loaded\n");
> @@ -713,6 +731,7 @@ static int swsusp_header_init(void)
> swsusp_header = (struct swsusp_header*) __get_free_page(GFP_KERNEL);
> if (!swsusp_header)
> panic("Could not allocate memory for swsusp_header\n");
> + hibernate_io_ops = &swap_ops;
> return 0;
> }
>
>