[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200609262204.03512.rjw@sisk.pl>
Date: Tue, 26 Sep 2006 22:04:02 +0200
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Andrew Morton <akpm@...l.org>
Cc: Dave Jones <davej@...hat.com>, Pavel Machek <pavel@....cz>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -mm 3/6] swsusp: Use block device offsets to identify swap locations
On Tuesday, 26 September 2006 21:38, Andrew Morton wrote:
> On Sat, 23 Sep 2006 12:04:25 +0200
> "Rafael J. Wysocki" <rjw@...k.pl> wrote:
>
> > Make swsusp use block device offsets instead of swap offsets to identify swap
> > locations and make it use the same code paths for writing as well as for
> > reading data.
> >
> > This allows us to use the same code for handling swap files and swap
> > partitions and to simplify the code, eg. by dropping rw_swap_page_sync().
> >
> > ..
> >
> > +sector_t swapdev_block(int swap_type, pgoff_t offset)
>
> swapdev_block() returns sector_t.
>
> > -unsigned long alloc_swap_page(int swap, struct bitmap_page *bitmap)
> > +loff_t alloc_swapdev_block(int swap, struct bitmap_page *bitmap)
> > {
> > unsigned long offset;
> >
> > offset = swp_offset(get_swap_page_of_type(swap));
> > if (offset) {
> > - if (bitmap_set(bitmap, offset)) {
> > + if (bitmap_set(bitmap, offset))
> > swap_free(swp_entry(swap, offset));
> > - offset = 0;
> > - }
> > + else
> > + return swapdev_block(swap, offset);
> > }
> > - return offset;
> > + return 0;
> > }
>
> But alloc_swapdev_block() returns loff_t.
I'll change all that to sector_t.
> > void free_all_swap_pages(int swap, struct bitmap_page *bitmap)
> > Index: linux-2.6.18-rc7-mm1/kernel/power/user.c
> > ===================================================================
> > --- linux-2.6.18-rc7-mm1.orig/kernel/power/user.c
> > +++ linux-2.6.18-rc7-mm1/kernel/power/user.c
> > @@ -239,7 +239,7 @@ static int snapshot_ioctl(struct inode *
> > break;
> > }
> > }
> > - offset = alloc_swap_page(data->swap, data->bitmap);
> > + offset = alloc_swapdev_block(data->swap, data->bitmap);
>
> `offset' is declared loff_t, yet it is holding a sector_t.
>
> > ===================================================================
> > --- linux-2.6.18-rc7-mm1.orig/kernel/power/swap.c
> > +++ linux-2.6.18-rc7-mm1/kernel/power/swap.c
> > @@ -34,8 +34,8 @@ extern char resume_file[];
> > #define SWSUSP_SIG "S1SUSPEND"
> >
> > static struct swsusp_header {
> > - char reserved[PAGE_SIZE - 20 - sizeof(swp_entry_t)];
> > - swp_entry_t image;
> > + char reserved[PAGE_SIZE - 20 - sizeof(loff_t)];
> > + loff_t image;
>
> More possible sector_t/loff_t confusion.
>
> > static int swsusp_swap_check(void) /* This is called before saving image */
> > {
> > - int res = swap_type_of(swsusp_resume_device, 0);
> > + int res;
> >
> > - if (res >= 0) {
> > - root_swap = res;
> > - return 0;
> > - }
> > - return res;
> > + res = swap_type_of(swsusp_resume_device, 0);
> > + if (res < 0)
> > + return res;
> > +
> > + root_swap = res;
> > + resume_bdev = open_by_devnum(swsusp_resume_device, FMODE_WRITE);
> > + if (IS_ERR(resume_bdev))
> > + return PTR_ERR(resume_bdev);
> > +
> > + set_blocksize(resume_bdev, PAGE_SIZE);
> > + return 0;
> > }
>
> set_blocksize() can fail.
Will check for that.
>
> > -#define MAP_PAGE_ENTRIES (PAGE_SIZE / sizeof(long) - 1)
> > +#define MAP_PAGE_ENTRIES (PAGE_SIZE / sizeof(loff_t) - 1)
>
> I think this is dealing with sector_t's?
-
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