From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>,
Linus Torvalds <torvalds@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-pm@vger.kernel.org
Subject: Re: Regression: hibernation is broken since e6bc9de714972cac34daa1dc1567ee48a47a9342
Date: Tue, 18 Feb 2020 21:03:14 -0800 [thread overview]
Message-ID: <20200219050314.GA9504@magnolia> (raw)
In-Reply-To: <20200214211523.GA32637@dumbo>
On Fri, Feb 14, 2020 at 10:15:24PM +0100, Domenico Andreoli wrote:
> [ added linux-pm ]
>
> On Thu, Feb 13, 2020 at 11:41:35AM -0800, Darrick J. Wong wrote:
> > On Thu, Feb 13, 2020 at 11:34:10AM -0800, Darrick J. Wong wrote:
> > >
> > > 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.
As I suspected, the in-kernel hibernate can write the memory image to
the swap file just fine...
> > > 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))
...but maybe we could do something a little more targeted?
> 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.
Hmm. I was poking around in the uswsusp code and I /think/ it prepares
the kernel for the userspace-driven hibernation by calling the ioctl
SNAPSHOT_SET_SWAP_FILE on the device that it's going to use. If that's
true, we could have it clear S_SWAPFILE on the chosen device, though I'd
have to do some more digging to figure out how to restore the flag after
resuming from hibernation.
--D
>
> --- 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))
>
> > 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.
>
> Dom
>
> --
> rsa4096: 3B10 0CA1 8674 ACBA B4FE FCD2 CE5B CF17 9960 DE13
> ed25519: FFB4 0CC3 7F2E 091D F7DA 356E CC79 2832 ED38 CB05
next prev parent reply other threads:[~2020-02-19 5:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-13 17:23 Regression: hibernation is broken since e6bc9de714972cac34daa1dc1567ee48a47a9342 Domenico Andreoli
2020-02-13 17:57 ` Darrick J. Wong
2020-02-13 18:35 ` Domenico Andreoli
2020-02-13 19:34 ` Darrick J. Wong
2020-02-13 19:41 ` Darrick J. Wong
2020-02-14 21:15 ` Domenico Andreoli
2020-02-19 5:03 ` Darrick J. Wong [this message]
2020-02-22 0:23 ` Darrick J. Wong
2020-02-23 19:03 ` Domenico Andreoli
2020-02-25 20:26 ` Darrick J. Wong
2020-03-17 15:51 ` Brad Campbell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200219050314.GA9504@magnolia \
--to=darrick.wong@oracle.com \
--cc=hch@lst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.