Re: [PATCH -mm 3/6] swsusp: Use block device offsets to identify swap locations

[Andrew Morton <akpm@osdl.org>]

On Sat, 23 Sep 2006 12:04:25 +0200
"Rafael J. Wysocki" <rjw@sisk.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.

>  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.

> -#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?