| 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: <201003252338.48513.rjw@sisk.pl>
Date: Thu, 25 Mar 2010 23:38:48 +0100
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Jiri Slaby <jslaby@...e.cz>
Cc: jirislaby@...il.com, pavel@....cz,
linux-pm@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
Nigel Cunningham <ncunningham@...a.org.au>
Subject: Re: [RFC 11/15] PM / Hibernate: add chunk i/o support
On Tuesday 23 March 2010, Jiri Slaby wrote:
> Chunk support is useful when not writing whole pages at once. It takes
> care of joining the buffers into the pages and writing at once when
> needed.
>
> In the end when pages are compressed they use this interface as well
> (because they are indeed shorter than PAGE_SIZE).
>
> Signed-off-by: Jiri Slaby <jslaby@...e.cz>
> Cc: Nigel Cunningham <ncunningham@...a.org.au>
> Cc: "Rafael J. Wysocki" <rjw@...k.pl>
> ---
> kernel/power/block_io.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++
> kernel/power/power.h | 6 +++
> 2 files changed, 87 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/power/block_io.c b/kernel/power/block_io.c
> index 2b7898a..37412f7 100644
> --- a/kernel/power/block_io.c
> +++ b/kernel/power/block_io.c
> @@ -3,6 +3,7 @@
> *
> * Copyright (C) 1998,2001-2005 Pavel Machek <pavel@...e.cz>
> * Copyright (C) 2006 Rafael J. Wysocki <rjw@...k.pl>
> + * Copyright (C) 2004-2008 Nigel Cunningham (nigel at tuxonice net)
> *
> * This file is released under the GPLv2.
> */
> @@ -14,6 +15,9 @@
>
> #include "power.h"
>
> +static char *sws_writer_buffer;
> +static unsigned long sws_writer_buffer_pos;
> +
> /**
> * submit - submit BIO request.
> * @rw: READ or WRITE.
> @@ -74,6 +78,83 @@ int sws_bio_write_page(pgoff_t page_off, void *addr, struct bio **bio_chain)
> virt_to_page(addr), bio_chain);
> }
>
> +int sws_rw_buffer_init(int writing)
> +{
> + BUG_ON(sws_writer_buffer || sws_writer_buffer_pos);
Please don't do that. Fail the operation instead. You can also use WARN_ON
or WARN if you _really_ want the user to notice the failure.
BUG_ON's like this are annoying like hell for testers who trigger them.
> + sws_writer_buffer = (void *)get_zeroed_page(__GFP_WAIT | __GFP_HIGH);
> + if (sws_writer_buffer && !writing)
> + sws_writer_buffer_pos = PAGE_SIZE;
> +
> + return sws_writer_buffer ? 0 : -ENOMEM;
> +}
> +
> +/**
> + * sws_rw_buffer - combine smaller buffers into PAGE_SIZE I/O
> + * @writing: Bool - whether writing (or reading).
> + * @buffer: The start of the buffer to write or fill.
> + * @buffer_size: The size of the buffer to write or fill.
> + **/
> +int sws_rw_buffer(int writing, char *buffer, int buffer_size)
> +{
> + int bytes_left = buffer_size, ret;
> +
> + while (bytes_left) {
> + char *source_start = buffer + buffer_size - bytes_left;
> + char *dest_start = sws_writer_buffer + sws_writer_buffer_pos;
> + int capacity = PAGE_SIZE - sws_writer_buffer_pos;
> + char *to = writing ? dest_start : source_start;
> + char *from = writing ? source_start : dest_start;
> +
> + if (bytes_left <= capacity) {
> + memcpy(to, from, bytes_left);
> + sws_writer_buffer_pos += bytes_left;
> + return 0;
> + }
> +
> + /* Complete this page and start a new one */
> + memcpy(to, from, capacity);
> + bytes_left -= capacity;
> +
> + if (writing) {
> + ret = sws_io_ops->write_page(sws_writer_buffer, NULL);
> + clear_page(sws_writer_buffer);
Why do we need that clear_page()?
> + if (ret)
> + return ret;
Please move these two lines out of the if()/else.
> + } else {
> + ret = sws_io_ops->read_page(sws_writer_buffer, NULL);
> + if (ret)
> + return ret;
> + }
> +
> + sws_writer_buffer_pos = 0;
> + }
> +
> + return 0;
> +}
> +
> +int sws_rw_buffer_flush_page(int writing)
> +{
> + int ret = 0;
> + if (writing && sws_writer_buffer_pos)
> + ret = sws_io_ops->write_page(sws_writer_buffer, NULL);
> + sws_writer_buffer_pos = writing ? 0 : PAGE_SIZE;
> + return ret;
> +}
I'd split the above into two functions, one for writing and the other for
reading.
Doing the same with sws_rw_buffer() (under a better name), for the sake of
clarity, also might make some sense, apparently.
> +
> +int sws_rw_buffer_finish(int writing)
> +{
> + int ret = 0;
> +
> + ret = sws_rw_buffer_flush_page(writing);
> +
> + free_page((unsigned long)sws_writer_buffer);
> + sws_writer_buffer = NULL;
> + sws_writer_buffer_pos = 0;
> +
> + return ret;
> +}
> +
> int sws_wait_on_bio_chain(struct bio **bio_chain)
> {
> struct bio *bio;
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 0f08de4..50a888a 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -3,6 +3,8 @@
> #include <linux/utsname.h>
> #include <linux/freezer.h>
>
> +struct swap_map_handle;
> +
> struct swsusp_info {
> struct new_utsname uts;
> u32 version_code;
> @@ -161,6 +163,10 @@ extern int sws_bio_read_page(pgoff_t page_off, void *addr,
> extern int sws_bio_write_page(pgoff_t page_off, void *addr,
> struct bio **bio_chain);
> extern int sws_wait_on_bio_chain(struct bio **bio_chain);
> +extern int sws_rw_buffer_init(int writing);
> +extern int sws_rw_buffer_finish(int writing);
> +extern int sws_rw_buffer_flush_page(int writing);
> +extern int sws_rw_buffer(int writing, char *buffer, int buffer_size);
>
> struct timeval;
> /* kernel/power/swsusp.c */
>
--
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