| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <201006241720.52879.rjw@sisk.pl>
Date: Thu, 24 Jun 2010 17:20:52 +0200
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Jiri Slaby <jslaby@...e.cz>
Cc: pavel@....cz, linux-pm@...ts.linux-foundation.org,
linux-kernel@...r.kernel.org, jirislaby@...il.com
Subject: Re: [PATCH 1/9] PM / Hibernate: swap, switch to hibernate_io_handle
On Wednesday, June 02, 2010, Jiri Slaby wrote:
> Hi,
Hi,
> I addressed the comments I got on the previous RFC.
Well, some of them. :-)
> I left the handles in place, the functions in hibernate_io_ops now works on
> them. Further I got rid of the memory barriers and minimized global variables
> as much as possible. Comments welcome.
One general comment first. Can we just simply assume that the kernel won't
do _any_ transformations of image data in the case when s2disk is used?
I think that's going to simplify things quite a bit.
> --
>
> Some code, which will be moved out of swap.c, will know nothing about
> swap. There will be also other than swap writers later, so that it
> won't make sense at all.
>
> So introduce a new structure called hibernate_io_handle which
> currently contains only a pointer to private data, but is independent
> on I/O reader/writer actually used. Private data are swap_map_handle
> for now.
>
> This structure is allocated in _start and freed in _finish. This will
> correspond to the later introduction of hibernate_io_ops where users
> will do handle=start->repeat{read/write(handle)}->finish(handle).
> I.e. they will operate on handle instead of global variables.
OK
This one looks good.
> Signed-off-by: Jiri Slaby <jslaby@...e.cz>
> Cc: "Rafael J. Wysocki" <rjw@...k.pl>
> ---
> kernel/power/power.h | 24 ++++++++++
> kernel/power/swap.c | 114 +++++++++++++++++++++++++++++++------------------
> 2 files changed, 96 insertions(+), 42 deletions(-)
>
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 006270f..7427d54 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -1,3 +1,4 @@
> +#include <linux/slab.h> /* kzalloc */
> #include <linux/suspend.h>
> #include <linux/suspend_ioctls.h>
> #include <linux/utsname.h>
> @@ -115,6 +116,29 @@ struct snapshot_handle {
> */
> #define data_of(handle) ((handle).buffer)
>
> +/**
> + * struct hibernate_io_handle - handle for image I/O processing
> + *
> + * @priv: private data for each processor (swap_map_handle etc.)
> + */
> +struct hibernate_io_handle {
> + void *priv;
> +};
> +
> +/**
> + * hib_io_handle_alloc - allocate io handle with priv_size for private data
> + *
> + * @priv_size: the sie to allocate behind hibernate_io_handle for private use
> + */
> +static inline struct hibernate_io_handle *hib_io_handle_alloc(size_t priv_size)
> +{
> + struct hibernate_io_handle *ret;
> + ret = kzalloc(sizeof(*ret) + priv_size, GFP_KERNEL);
> + if (ret)
> + ret->priv = ret + 1;
> + return ret;
> +}
> +
> 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);
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index b0bb217..7096d20 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -268,8 +268,10 @@ static void release_swap_writer(struct swap_map_handle *handle)
> handle->cur = NULL;
> }
>
> -static int get_swap_writer(struct swap_map_handle *handle)
> +static struct hibernate_io_handle *get_swap_writer(void)
> {
> + struct hibernate_io_handle *io_handle;
> + struct swap_map_handle *handle;
> int ret;
>
> ret = swsusp_swap_check();
> @@ -277,12 +279,18 @@ static int get_swap_writer(struct swap_map_handle *handle)
> if (ret != -ENOSPC)
> printk(KERN_ERR "PM: Cannot find swap device, try "
> "swapon -a.\n");
> - return ret;
> + return ERR_PTR(ret);
> }
> + io_handle = hib_io_handle_alloc(sizeof(*handle));
> + if (!io_handle) {
> + ret = -ENOMEM;
> + goto err_close;
> + }
> + handle = io_handle->priv;
> handle->cur = (struct swap_map_page *)get_zeroed_page(GFP_KERNEL);
> if (!handle->cur) {
> ret = -ENOMEM;
> - goto err_close;
> + goto err_free;
> }
> handle->cur_swap = alloc_swapdev_block(root_swap);
> if (!handle->cur_swap) {
> @@ -291,17 +299,20 @@ static int get_swap_writer(struct swap_map_handle *handle)
> }
> handle->k = 0;
> handle->first_sector = handle->cur_swap;
> - return 0;
> + return io_handle;
> err_rel:
> release_swap_writer(handle);
> err_close:
> swsusp_close(FMODE_WRITE);
> - return ret;
> +err_free:
> + kfree(io_handle);
> + return ERR_PTR(ret);
> }
>
> -static int swap_write_page(struct swap_map_handle *handle, void *buf,
> - struct bio **bio_chain)
> +static int swap_write_page(struct hibernate_io_handle *io_handle, void *buf,
> + struct bio **bio_chain)
> {
> + struct swap_map_handle *handle = io_handle->priv;
> int error = 0;
> sector_t offset;
>
> @@ -339,9 +350,11 @@ static int flush_swap_writer(struct swap_map_handle *handle)
> return -EINVAL;
> }
>
> -static int swap_writer_finish(struct swap_map_handle *handle,
> +static int swap_writer_finish(struct hibernate_io_handle *io_handle,
> unsigned int flags, int error)
> {
> + struct swap_map_handle *handle = io_handle->priv;
> +
> if (!error) {
> flush_swap_writer(handle);
> printk(KERN_INFO "PM: S");
> @@ -352,6 +365,7 @@ static int swap_writer_finish(struct swap_map_handle *handle,
> if (error)
> free_all_swap_pages(root_swap);
> release_swap_writer(handle);
> + kfree(io_handle);
> swsusp_close(FMODE_WRITE);
>
> return error;
> @@ -361,8 +375,8 @@ static int swap_writer_finish(struct swap_map_handle *handle,
> * save_image - save the suspend image data
> */
>
> -static int save_image(struct swap_map_handle *handle,
> - struct snapshot_handle *snapshot,
> +static int save_image(struct hibernate_io_handle *io_handle,
> + struct snapshot_handle *snapshot,
> unsigned int nr_to_write)
> {
> unsigned int m;
> @@ -385,7 +399,7 @@ static int save_image(struct swap_map_handle *handle,
> ret = snapshot_read_next(snapshot);
> if (ret <= 0)
> break;
> - ret = swap_write_page(handle, data_of(*snapshot), &bio);
> + ret = swap_write_page(io_handle, data_of(*snapshot), &bio);
> if (ret)
> break;
> if (!(nr_pages % m))
> @@ -431,17 +445,17 @@ static int enough_swap(unsigned int nr_pages)
>
> int swsusp_write(unsigned int flags)
> {
> - struct swap_map_handle handle;
> + struct hibernate_io_handle *io_handle;
> struct snapshot_handle snapshot;
> struct swsusp_info *header;
> unsigned long pages;
> int error;
>
> pages = snapshot_get_image_size();
> - error = get_swap_writer(&handle);
> - if (error) {
> + io_handle = get_swap_writer();
> + if (IS_ERR(io_handle)) {
> printk(KERN_ERR "PM: Cannot get swap writer\n");
> - return error;
> + return PTR_ERR(io_handle);
> }
> if (!enough_swap(pages)) {
> printk(KERN_ERR "PM: Not enough free swap\n");
> @@ -457,11 +471,11 @@ int swsusp_write(unsigned int flags)
> goto out_finish;
> }
> header = (struct swsusp_info *)data_of(snapshot);
> - error = swap_write_page(&handle, header, NULL);
> + error = swap_write_page(io_handle, header, NULL);
> if (!error)
> - error = save_image(&handle, &snapshot, pages - 1);
> + error = save_image(io_handle, &snapshot, pages - 1);
> out_finish:
> - error = swap_writer_finish(&handle, flags, error);
> + error = swap_writer_finish(io_handle, flags, error);
> return error;
> }
>
> @@ -477,32 +491,43 @@ static void release_swap_reader(struct swap_map_handle *handle)
> handle->cur = NULL;
> }
>
> -static int get_swap_reader(struct swap_map_handle *handle,
> - unsigned int *flags_p)
> +static struct hibernate_io_handle *get_swap_reader(unsigned int *flags_p)
> {
> + struct hibernate_io_handle *io_handle;
> + struct swap_map_handle *handle;
> int error;
>
> *flags_p = swsusp_header->flags;
>
> if (!swsusp_header->image) /* how can this happen? */
> - return -EINVAL;
> + return ERR_PTR(-EINVAL);
>
> + io_handle = hib_io_handle_alloc(sizeof(*handle));
> + if (!io_handle)
> + return ERR_PTR(-ENOMEM);
> + handle = io_handle->priv;
> handle->cur = (struct swap_map_page *)get_zeroed_page(__GFP_WAIT | __GFP_HIGH);
> - if (!handle->cur)
> - return -ENOMEM;
> + if (!handle->cur) {
> + error = -ENOMEM;
> + goto err_free;
> + }
>
> error = hib_bio_read_page(swsusp_header->image, handle->cur, NULL);
> - if (error) {
> - release_swap_reader(handle);
> - return error;
> - }
> + if (error)
> + goto err_rel;
> handle->k = 0;
> - return 0;
> + return io_handle;
> +err_rel:
> + release_swap_reader(handle);
> +err_free:
> + kfree(io_handle);
> + return ERR_PTR(error);
> }
>
> -static int swap_read_page(struct swap_map_handle *handle, void *buf,
> - struct bio **bio_chain)
> +static int swap_read_page(struct hibernate_io_handle *io_handle, void *buf,
> + struct bio **bio_chain)
> {
> + struct swap_map_handle *handle = io_handle->priv;
> sector_t offset;
> int error;
>
> @@ -526,22 +551,25 @@ static int swap_read_page(struct swap_map_handle *handle, void *buf,
> return error;
> }
>
> -static int swap_reader_finish(struct swap_map_handle *handle)
> +static int swap_reader_finish(struct hibernate_io_handle *io_handle)
> {
> + struct swap_map_handle *handle = io_handle->priv;
> +
> release_swap_reader(handle);
> + kfree(io_handle);
>
> return 0;
> }
>
> /**
> - * load_image - load the image using the swap map handle
> + * load_image - load the image
> * @handle and the snapshot handle @snapshot
> * (assume there are @nr_pages pages to load)
> */
>
> -static int load_image(struct swap_map_handle *handle,
> - struct snapshot_handle *snapshot,
> - unsigned int nr_to_read)
> +static int load_image(struct hibernate_io_handle *io_handle,
> + struct snapshot_handle *snapshot,
> + unsigned int nr_to_read)
> {
> unsigned int m;
> int error = 0;
> @@ -563,7 +591,7 @@ static int load_image(struct swap_map_handle *handle,
> error = snapshot_write_next(snapshot);
> if (error <= 0)
> break;
> - error = swap_read_page(handle, data_of(*snapshot), &bio);
> + error = swap_read_page(io_handle, data_of(*snapshot), &bio);
> if (error)
> break;
> if (snapshot->sync_read)
> @@ -598,7 +626,7 @@ static int load_image(struct swap_map_handle *handle,
> int swsusp_read(unsigned int *flags_p)
> {
> int error;
> - struct swap_map_handle handle;
> + struct hibernate_io_handle *io_handle;
> struct snapshot_handle snapshot;
> struct swsusp_info *header;
>
> @@ -607,14 +635,16 @@ int swsusp_read(unsigned int *flags_p)
> if (error < PAGE_SIZE)
> return error < 0 ? error : -EFAULT;
> header = (struct swsusp_info *)data_of(snapshot);
> - error = get_swap_reader(&handle, flags_p);
> - if (error)
> + io_handle = get_swap_reader(flags_p);
> + if (IS_ERR(io_handle)) {
> + error = PTR_ERR(io_handle);
> goto end;
> + }
> if (!error)
> - error = swap_read_page(&handle, header, NULL);
> + error = swap_read_page(io_handle, header, NULL);
> if (!error)
> - error = load_image(&handle, &snapshot, header->pages - 1);
> - swap_reader_finish(&handle);
> + error = load_image(io_handle, &snapshot, header->pages - 1);
> + swap_reader_finish(io_handle);
> end:
> if (!error)
> pr_debug("PM: Image successfully loaded\n");
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists