Re: Regression: hibernation is broken since e6bc9de714972cac34daa1dc1567ee48a47a9342

[ added linux-pm ]

On Thu, Feb 13, 2020 at 11:41:35AM -0800, Darrick J. Wong wrote:

quoted

On Thu, Feb 13, 2020 at 11:34:10AM -0800, Darrick J. Wong wrote:

quoted

Well ... you could try the in-kernel hibernate (which I think is what
'systemctl hibernate' does), though you'd lose the nifty features of
µswsusp.

Indeed 'systemctl hibernate' works perfectly with v5.6-rc1 in my setup.

quoted

quoted

In the end, though, I'll probably have to revert all those IS_SWAPFILE
checks (at least if CONFIG_HIBERNATION=y) since it's not fair to force
you to totally reconfigure your hibernation setup.

Also, does the following partial revert fix uswsusp for you?  It'll
allow the direct writes that uswsusp wants to do, while leaving the rest
(mmap writes) in place.

--D

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 69bf2fb6f7cd..077d9fa6b87d 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c

@@ -2001,8 +2001,10 @@ ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (bdev_read_only(I_BDEV(bd_inode)))
 		return -EPERM;
 
+#ifndef CONFIG_HIBERNATION
 	if (IS_SWAPFILE(bd_inode))
 		return -ETXTBSY;
+#endif

This alone is enough to make uswsusp work again.

I propose this alternative:

--- a/fs/block_dev.c
+++ b/fs/block_dev.c

@@ -2001,7 +2001,8 @@ ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
        if (bdev_read_only(I_BDEV(bd_inode)))
                return -EPERM;
 
-       if (IS_SWAPFILE(bd_inode))
+       /* Hibernation might happen via uswsusp, let it write to the swap */
+       if (IS_SWAPFILE(bd_inode) && !IS_ENABLED(CONFIG_HIBERNATION))
                return -ETXTBSY;
 
        if (!iov_iter_count(from))

I looked for a more selective way to enable writes to swap at runtime,
so I tried with system_entering_hibernation() but it's not yet armed
at the point in which uswsusp wants to write to the swap and therefore
it does not work.

--- a/fs/block_dev.c
+++ b/fs/block_dev.c

@@ -34,6 +34,7 @@
 #include <linux/task_io_accounting_ops.h>
 #include <linux/falloc.h>
 #include <linux/uaccess.h>
+#include <linux/suspend.h>
 #include "internal.h"

 struct bdev_inode {

@@ -2001,7 +2002,8 @@ ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
        if (bdev_read_only(I_BDEV(bd_inode)))
                return -EPERM;

-       if (IS_SWAPFILE(bd_inode))
+       /* Hibernation might happen via uswsusp, let it write to the swap */
+       if (IS_SWAPFILE(bd_inode) && !system_entering_hibernation())
                return -ETXTBSY;

        if (!iov_iter_count(from))

quoted

 	if (!iov_iter_count(from))
 		return 0;

diff --git a/mm/filemap.c b/mm/filemap.c
index 1784478270e1..3df3211abe25 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c

@@ -2920,8 +2920,10 @@ inline ssize_t generic_write_checks(struct kiocb *iocb, struct iov_iter *from)
 	loff_t count;
 	int ret;
 
+#ifndef CONFIG_HIBERNATION
 	if (IS_SWAPFILE(inode))
 		return -ETXTBSY;
+#endif
 
 	if (!iov_iter_count(from))
 		return 0;

The above is not needed in my case but I'm not sure it would not be
needed in some other configuration of uswsusp.