Re: [PATCH RFC 2/6] mm: convert PG_balloon to PG_offline

[Cc Konstantin - the patch is http://lkml.kernel.org/r/20181114211704.6381-3-david@redhat.com]

On Thu 15-11-18 10:21:13, David Hildenbrand wrote:

On 15.11.18 03:07, Mike Rapoport wrote:

quoted

On Wed, Nov 14, 2018 at 11:49:15PM +0100, David Hildenbrand wrote:

quoted

On 14.11.18 23:23, Matthew Wilcox wrote:

quoted

On Wed, Nov 14, 2018 at 10:17:00PM +0100, David Hildenbrand wrote:

quoted

Rename PG_balloon to PG_offline. This is an indicator that the page is
logically offline, the content stale and that it should not be touched
(e.g. a hypervisor would have to allocate backing storage in order for the
guest to dump an unused page).  We can then e.g. exclude such pages from
dumps.

In following patches, we will make use of this bit also in other balloon
drivers.  While at it, document PGTABLE.

Thank you for documenting PGTABLE.  I didn't realise I also had this
document to update when I added PGTABLE.

Thank you for looking into this :)

quoted

quoted

+++ b/Documentation/admin-guide/mm/pagemap.rst

@@ -78,6 +78,8 @@ number of times a page is mapped.
     23. BALLOON
     24. ZERO_PAGE
     25. IDLE
+    26. PGTABLE
+    27. OFFLINE

So the offline *user* bit is new ... even though the *kernel* bit
just renames the balloon bit.  I'm not sure how I feel about this.
I'm going to think about it some more.  Could you share your decision
process with us?

BALLOON was/is documented as

"23 - BALLOON
    balloon compaction page
"

and only includes all virtio-ballon pages after the non-lru migration
feature has been implemented for ballooned pages. Since then, this flag
does basically no longer stands for what it actually was supposed to do.

Perhaps I missing something, but how the user should interpret "23" when he
reads /proc/kpageflags?

Looking at the history in more detail:

commit 09316c09dde33aae14f34489d9e3d243ec0d5938
Author: Konstantin Khlebnikov [off-list ref]
Date:   Thu Oct 9 15:29:32 2014 -0700

    mm/balloon_compaction: add vmstat counters and kpageflags bit

    Always mark pages with PageBalloon even if balloon compaction is
disabled
    and expose this mark in /proc/kpageflags as KPF_BALLOON.


So KPF_BALLOON was exposed when virtio-balloon pages were always marked
with PG_balloon. So the documentation is actually wrong ("balloon page"
vs. "balloon compaction page").

I have no idea who actually used that information. I suspect this was
just some debugging aid.

quoted

quoted

To not break uapi I decided to not rename it but instead to add a new flag.

I've got a feeling that uapi was anyway changed for the BALLON flag
meaning.

Yes. If we *replace* KPF_BALLOON by KPF_OFFLINE

a) Some applications might no longer compile (I guess that's ok)
b) Some old applications will treat KPF_OFFLINE like KPF_BALLOON (which
should at least for virtio-balloon usage until now be fine - it is just
more generic)

I do not think any compilation could break. If the semantic of the flag
is preserved then everything should work as expected.
-- 
Michal Hocko
SUSE Labs