On 7/6/05, Nigel Cunningham <[EMAIL PROTECTED]> wrote:
> diff -ruNp 623-generic-block-io.patch-old/kernel/power/suspend_block_io.c
> 623-generic-block-io.patch-new/kernel/power/suspend_block_io.c
> --- 623-generic-block-io.patch-old/kernel/power/suspend_block_io.c
> 1970-01-01 10:00:00.000000000 +1000
> +++ 623-generic-block-io.patch-new/kernel/power/suspend_block_io.c
> 2005-07-05 23:48:59.000000000 +1000
> @@ -0,0 +1,817 @@
> +#include "suspend2_core/suspend.h"
> +#include "suspend2_core/proc.h"
> +#include "suspend2_core/plugins.h"
> +#include "suspend2_core/utility.h"
> +#include "suspend2_core/prepare_image.h"
You've got circular dependencies. kernel/power/suspend2_core depends
on kernel/power and vice versa. Please just consider putting them all
in kernel/power/
> +/* Bits in struct io_info->flags */
> +#define IO_WRITING 1
> +#define IO_RESTORE_PAGE_PROT 2
> +#define IO_AWAITING_READ 3
> +#define IO_AWAITING_WRITE 4
> +#define IO_AWAITING_SUBMIT 5
> +#define IO_AWAITING_CLEANUP 6
> +#define IO_HANDLE_PAGE_PROT 7
Please use enums instead.
> +
> +#define MAX_OUTSTANDING_IO 1024
> +
> +/*
> + * ---------------------------------------------------------------
> + *
> + * IO in progress information storage and helpers
> + *
> + * ---------------------------------------------------------------
> + */
Could we please drop the above banner. Hurts my eyes...
> +
> +struct io_info {
> + struct bio * sys_struct;
> + long block[PAGE_SIZE/512];
You're hardcoding sector size here, no?
> + struct page * buffer_page;
> + struct page * data_page;
> + unsigned long flags;
> + struct block_device * dev;
> + struct list_head list;
> + int readahead_index;
> + struct work_struct work;
> +};
> +
> +/* Locks separated to allow better SMP support.
> + * An io_struct moves through the lists as follows.
> + * free -> submit_batch -> busy -> ready_for_cleanup -> free
> + */
> +static LIST_HEAD(ioinfo_free);
> +static spinlock_t ioinfo_free_lock = SPIN_LOCK_UNLOCKED;
Please use DEFINE_SPINLOCK instead. It is preferred as some automatic
lock checkers need it.
> +#define BITS_PER_UL (8 * sizeof(unsigned long))
> +static volatile unsigned long suspend_readahead_flags[(MAX_READAHEAD +
> BITS_PER_UL - 1) / BITS_PER_UL];
<asm/types.h> has BITS_PER_LONG. Use it.
> +static int suspend_end_bio(struct bio * bio, unsigned int num, int err)
> +{
> + struct io_info *io_info = (struct io_info *) bio->bi_private;
Redundant cast.
> +static void suspend_wait_on_readahead(int readahead_index)
> +{
> + int index = readahead_index/(8 * sizeof(unsigned long));
> + int bit = readahead_index - index * 8 * sizeof(unsigned long);
Use BITS_PER_LONG here.
> +
> + /* read_ahead_index is the one we want to return */
> + while (!test_bit(bit, &suspend_readahead_flags[index]))
> + do_bio_wait(6);
> +}
> +
> +/*
> + * readahead_done
> + *
> + * Returns whether the readahead requested is ready.
> + */
> +
> +static int suspend_readahead_ready(int readahead_index)
> +{
> + int index = readahead_index/(8 * sizeof(unsigned long));
> + int bit = readahead_index - (index * 8 * sizeof(unsigned long));
Ditto.
> +
> + return test_bit(bit, &suspend_readahead_flags[index]);
> +}
> +
> +/* suspend_readahead_prepare
> + * Set up for doing readahead on an image */
> +static int suspend_prepare_readahead(int index)
> +{
> + unsigned long new_page = get_zeroed_page(GFP_ATOMIC);
> +
> + if(!new_page)
> + return -ENOMEM;
> +
> + suspend_bio_ops.readahead_pages[index] = virt_to_page(new_page);
> + return 0;
> +}
> +
> +/* suspend_readahead_cleanup
> + * Clean up structures used for readahead */
> +static void suspend_cleanup_readahead(int page)
> +{
> + __free_pages(suspend_bio_ops.readahead_pages[page], 0);
> + suspend_bio_ops.readahead_pages[page] = 0;
> + return;
> +}
> +
> +static unsigned long suspend_bio_memory_needed(void)
> +{
> + /* We want to have at least enough memory so as to have 128 I/O
> + * transactions on the fly at once. If we can to more, fine. */
> + return (128 * (PAGE_SIZE + sizeof(struct request) +
> + sizeof(struct bio) + sizeof(struct io_info)));
Where did the magic 128 come from?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
