Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210630100432.v1.1.I09866d90c6de14f21223a03e9e6a31f8a02ecbaf@changeid>
 <20210701200253.GB983@bug>
In-Reply-To: <20210701200253.GB983@bug>
From:   Evan Green <evgreen@chromium.org>
Date:   Thu, 1 Jul 2021 16:04:57 -0700
Message-ID: <CAE=gft6UUesGAcT3cweFuQBOh3xFzN2gCDNGWZRqizNmPT7Rdg@mail.gmail.com>
Subject: Re: [PATCH v1] mm: Enable suspend-only swap spaces
To:     Pavel Machek <pavel@ucw.cz>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Alex Shi <alexs@kernel.org>,
        Alistair Popple <apopple@nvidia.com>,
        David Hildenbrand <david@redhat.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>,
        Stephen Rothwell <sfr@canb.auug.org.au>,
        Vlastimil Babka <vbabka@suse.cz>,
        LKML <linux-kernel@vger.kernel.org>, linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jul 1, 2021 at 1:02 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
>
> > 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.
> >
> > Signed-off-by: Evan Green <evgreen@chromium.org>
>
> Makes sense to me.
>
> Reviewed-by: Pavel Machek <pavel@ucw.cz>

Thanks!

>
> >  #define SWAP_FLAG_DISCARD_ONCE       0x20000 /* discard swap area at swapon-time */
> >  #define SWAP_FLAG_DISCARD_PAGES 0x40000 /* discard page-clusters after use */
> > +#define SWAP_FLAG_NOSWAP     0x80000 /* use only for suspend, not swap */
>
> I'd say "only for hibernation". And actually maybe code would be more clear if logic was reverted.

Sure about the rewording. Yes, I also thought about flipping the
polarity. This made more sense to me as an outlier condition, despite
the slight awkwardness of a negative flag. And the usermode flag has
to be written this way, so I might as well carry it through. I think
I'll keep it unless anyone feels strongly.

>
> Aha, and you may want to check... does the hibernation still work for you without the swap?
>
> Because we need half memory free to create swap image and swap is really quite useful for that.

Yes, hibernation still works. You're right that without another swap
space set up, it starts to fail when half of memory is used up. This
flag gives me the control to exclusively steer swap towards one device
and hibernate towards another.
-Evan