Re: 2.6.20 regression: suspend to disk no more works

[Andrey Borzenkov]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tuesday 02 January 2007 02:28, Rafael J. Wysocki wrote:
> Hi,
>
> On Monday, 1 January 2007 20:44, Andrey Borzenkov wrote:
> > In *the same* configuration STD now fails with "Cannot find swap device".
> > The reason is changes in kernel/power/swap.c. In 2.6.19 it did not
> > require valid swsusp_resume_device at all - it took first available swap
> > device and saved image. Later during resume swsusp_resume_device was set
> > either by command line or sysfs and everything worked nicely.
> >
> > Now swsusp_swap_check() unfortunately checks for swsusp_resume_device at
> > *suspend* time:
> >
> >         res = swap_type_of(swsusp_resume_device, swsusp_resume_block);
> >         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);
> >
> > but in case of modular driver for swap device this is likely to be
> > undefined. This is as of 2.6.20-rc3.
>
> Actually, if you look at the 2.6.19 code, the call to swap_type_of() is
> there in swsusp_swap_check() too.
>
> The problem is with open_by_devnum(), I think, which obviously cannot
> succeed if swsusp_resume_device is not set.  I think we should return
> resume_bdev from swsusp_swap_check() like in the appended patch (untested).
>  Pavel?
>

this works, thank you

- -andrey

> > I already have seen these reports.
>
> Yes, me too.
>
> > While 'echo a:b > /sys/power/resume' before
> > suspend is a workaround, this still breaks perfectly valid setup that
> > worked before. Also 'echo a:b > /sys/power/resume' is actually wrong - we
> > are not going to resume at this point; but there is no way to just tell
> > kernel "use this device for next STD" ... also the error message is
> > misleading, it should complaint "no resume device found". Swap is there
> > all right.
>
> Thanks for the report.
>
> Greetings,
> Rafael
>
>
> ---
>  include/linux/swap.h |    2 +-
>  kernel/power/swap.c  |    9 +++++----
>  kernel/power/user.c  |    7 ++++---
>  mm/swapfile.c        |    8 +++++++-
>  4 files changed, 17 insertions(+), 9 deletions(-)
>
> Index: linux-2.6.20-rc3/include/linux/swap.h
> ===================================================================
> --- linux-2.6.20-rc3.orig/include/linux/swap.h
> +++ linux-2.6.20-rc3/include/linux/swap.h
> @@ -245,7 +245,7 @@ extern int swap_duplicate(swp_entry_t);
>  extern int valid_swaphandles(swp_entry_t, unsigned long *);
>  extern void swap_free(swp_entry_t);
>  extern void free_swap_and_cache(swp_entry_t);
> -extern int swap_type_of(dev_t, sector_t);
> +extern int swap_type_of(dev_t, sector_t, struct block_device **);
>  extern unsigned int count_swap_pages(int, int);
>  extern sector_t map_swap_page(struct swap_info_struct *, pgoff_t);
>  extern sector_t swapdev_block(int, pgoff_t);
> Index: linux-2.6.20-rc3/kernel/power/swap.c
> ===================================================================
> --- linux-2.6.20-rc3.orig/kernel/power/swap.c
> +++ linux-2.6.20-rc3/kernel/power/swap.c
> @@ -165,14 +165,15 @@ static int swsusp_swap_check(void) /* Th
>  {
>  	int res;
>
> -	res = swap_type_of(swsusp_resume_device, swsusp_resume_block);
> +	res = swap_type_of(swsusp_resume_device, swsusp_resume_block,
> +			&resume_bdev);
>  	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);
> +	res = blkdev_get(resume_bdev, FMODE_WRITE, O_RDWR);
> +	if (res)
> +		return res;
>
>  	res = set_blocksize(resume_bdev, PAGE_SIZE);
>  	if (res < 0)
> Index: linux-2.6.20-rc3/mm/swapfile.c
> ===================================================================
> --- linux-2.6.20-rc3.orig/mm/swapfile.c
> +++ linux-2.6.20-rc3/mm/swapfile.c
> @@ -434,7 +434,7 @@ void free_swap_and_cache(swp_entry_t ent
>   *
>   * This is needed for the suspend to disk (aka swsusp).
>   */
> -int swap_type_of(dev_t device, sector_t offset)
> +int swap_type_of(dev_t device, sector_t offset, struct block_device
> **bdev_p) {
>  	struct block_device *bdev = NULL;
>  	int i;
> @@ -450,6 +450,9 @@ int swap_type_of(dev_t device, sector_t
>  			continue;
>
>  		if (!bdev) {
> +			if (bdev_p)
> +				*bdev_p = sis->bdev;
> +
>  			spin_unlock(&swap_lock);
>  			return i;
>  		}
> @@ -459,6 +462,9 @@ int swap_type_of(dev_t device, sector_t
>  			se = list_entry(sis->extent_list.next,
>  					struct swap_extent, list);
>  			if (se->start_block == offset) {
> +				if (bdev_p)
> +					*bdev_p = sis->bdev;
> +
>  				spin_unlock(&swap_lock);
>  				bdput(bdev);
>  				return i;
> Index: linux-2.6.20-rc3/kernel/power/user.c
> ===================================================================
> --- linux-2.6.20-rc3.orig/kernel/power/user.c
> +++ linux-2.6.20-rc3/kernel/power/user.c
> @@ -58,7 +58,7 @@ static int snapshot_open(struct inode *i
>  	memset(&data->handle, 0, sizeof(struct snapshot_handle));
>  	if ((filp->f_flags & O_ACCMODE) == O_RDONLY) {
>  		data->swap = swsusp_resume_device ?
> -				swap_type_of(swsusp_resume_device, 0) : -1;
> +			swap_type_of(swsusp_resume_device, 0, NULL) : -1;
>  		data->mode = O_RDONLY;
>  	} else {
>  		data->swap = -1;
> @@ -327,7 +327,8 @@ static int snapshot_ioctl(struct inode *
>  			 * so we need to recode them
>  			 */
>  			if (old_decode_dev(arg)) {
> -				data->swap = swap_type_of(old_decode_dev(arg), 0);
> +				data->swap = swap_type_of(old_decode_dev(arg),
> +							0, NULL);
>  				if (data->swap < 0)
>  					error = -ENODEV;
>  			} else {
> @@ -427,7 +428,7 @@ static int snapshot_ioctl(struct inode *
>  			swdev = old_decode_dev(swap_area.dev);
>  			if (swdev) {
>  				offset = swap_area.offset;
> -				data->swap = swap_type_of(swdev, offset);
> +				data->swap = swap_type_of(swdev, offset, NULL);
>  				if (data->swap < 0)
>  					error = -ENODEV;
>  			} else {
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)

iD8DBQFFmhn/R6LMutpd94wRAs/nAJ4kcMD1KidpfQtTwnxYCQTf/ZNEzwCZATSf
6OFWDF/9sui3vvwmwwe6dos=
=fYE3
-----END PGP SIGNATURE-----
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/