LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Michal Hocko <mhocko@suse.com>
Cc: Evan Green <evgreen@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Pavel Machek <pavel@ucw.cz>, Alex Shi <alexs@kernel.org>,
	Alistair Popple <apopple@nvidia.com>,
	Jens Axboe <axboe@kernel.dk>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>,
	Miaohe Lin <linmiaohe@huawei.com>,
	Minchan Kim <minchan@kernel.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-api@vger.kernel.org
Subject: Re: [PATCH v2] mm: Enable suspend-only swap spaces
Date: Wed, 14 Jul 2021 09:51:13 +0200	[thread overview]
Message-ID: <b84dfb7b-9ae7-8cd7-ce65-0b1952e30e8e@redhat.com> (raw)
In-Reply-To: <YO55ZIrgkLXI4BbS@dhcp22.suse.cz>

On 14.07.21 07:43, Michal Hocko wrote:
> On Mon 12-07-21 09:41:26, David Hildenbrand wrote:
>> On 12.07.21 09:03, Michal Hocko wrote:
>>> [Cc linux-api]
>>>
>>> On Fri 09-07-21 10:50:48, Evan Green wrote:
>>>> Currently it's not possible to enable hibernation without also enabling
>>>> generic swap for a given swap area. These two use cases are not the
>>>> same. For example there may be users who want to enable hibernation,
>>>> but whose drives don't have the write endurance for generic swap
>>>> activities.
>>>>
>>>> Add a new SWAP_FLAG_NOSWAP that adds a swap region but refuses to allow
>>>> generic swapping to it. This region can still be wired up for use in
>>>> suspend-to-disk activities, but will never have regular pages swapped to
>>>> it.
>>>
>>> Could you expand some more on why a strict exclusion is really
>>> necessary? I do understand that one might not want to have swap storage
>>> available all the time but considering that swapon is really a light
>>> operation so something like the following should be a reasonable
>>> workaround, no?
>>> 	swapon storage/file
>>> 	s2disk
>>> 	swapoff storage
>>
>> I'm certainly not a hibernation expert, but I'd guess this can also be
>> triggered by HW events, so from the kernel and not only from user space
>> where your workaround would apply.
> 
> Is there anything preventing such a HW event doing the equivalent of the
> above?
> 

Let's take a look at hibernate() callers:

drivers/mfd/tps65010.c: calls hibernate() from IRQ contex, based on HW
                         event
kernel/power/autosleep.c: calls hibernate() when it thinks it might be
  			  a good time to go to sleep
kernel/power/main.c: calls hibernate() triggered by userspace
kernel/reboot.c: calls hibernate() triggered by userspace

So on two paths, hibernate() is not under user space control and the 
sequence you propose does not apply. The kernel itself has no idea which 
swap space to activate before hibernating, that's a user space decision. 
And without this patch, user space cannot communicate that decision to 
the kernel without eventually also swapping.

However, I assume in most cases (e.g., ACPI events, power button 
pressed, ...) we always notify user space, which in return decides which 
action to take. Doing it under kernel control is more of a corner case I 
guess, so I do wonder if we really care about these setups.

Anyhow, the proposal here does not sound completely crazy to me, 
although it's unfortunate how we decided to mangle hibernation and 
swapping into the same mechanism originally; a different interface to 
active "hibernation only backends" would be cleaner than doing a "swapon 
..." without swapping. However, that will require much more work and I 
am not sure if it's worth it ...

-- 
Thanks,

David / dhildenb


  reply	other threads:[~2021-07-14  7:51 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09 17:50 Evan Green
2021-07-09 22:20 ` Andrew Morton
2021-07-09 23:23   ` Evan Green
2021-07-09 23:33     ` Andrew Morton
2021-07-12 21:36       ` Evan Green
2021-07-12  7:03 ` Michal Hocko
2021-07-12  7:41   ` David Hildenbrand
2021-07-14  5:43     ` Michal Hocko
2021-07-14  7:51       ` David Hildenbrand [this message]
2021-07-14  8:00         ` Michal Hocko
2021-07-27 12:04         ` Pavel Machek
2021-07-27 12:19           ` David Hildenbrand
2021-07-27 12:01     ` Pavel Machek
2021-07-12 21:32   ` Evan Green
2021-07-14  5:41     ` Michal Hocko
2021-07-14 22:39       ` Evan Green
2021-07-27 12:10         ` Pavel Machek

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=b84dfb7b-9ae7-8cd7-ce65-0b1952e30e8e@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexs@kernel.org \
    --cc=apopple@nvidia.com \
    --cc=axboe@kernel.dk \
    --cc=evgreen@chromium.org \
    --cc=hannes@cmpxchg.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linmiaohe@huawei.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=minchan@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=vbabka@suse.cz \
    --cc=willy@infradead.org \
    --subject='Re: [PATCH v2] mm: Enable suspend-only swap spaces' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).