lkml.org 
[lkml]   [2005]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] [46/48] Suspend2 2.1.9.8 for 2.6.12: 622-swapwriter.patch
    On 7/6/05, Nigel Cunningham <nigel@suspend2.net> 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 majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-07-06 10:18    [W:4.048 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site