linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
To: David Hildenbrand <david@redhat.com>
Cc: "Michal Hocko" <mhocko@kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"Greg KH" <gregkh@linuxfoundation.org>,
	"Jan Höppner" <hoeppner@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	linux-api@vger.kernel.org,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: Ways to deprecate /sys/devices/system/memory/memoryX/phys_device ?
Date: Tue, 22 Sep 2020 15:56:11 +0200	[thread overview]
Message-ID: <20200922155611.379373f7@thinkpad> (raw)
In-Reply-To: <21852ccb-bd06-9281-7c8e-485ec02f2883@redhat.com>

On Thu, 10 Sep 2020 12:20:34 +0200
David Hildenbrand <david@redhat.com> wrote:

> Hi everybody,
> 
> I was just exploring how /sys/devices/system/memory/memoryX/phys_device
> is/was used. It's one of these interfaces that most probably never
> should have been added but now we are stuck with it.
> 
> "phys_device" was used on s390x in older versions of lsmem[2]/chmem[3],
> back when they were still part of s390x-tools. They were later replaced
> [5] by the variants in linux-utils. For example, RHEL6 and RHEL7 contain
> lsmem/chmem from s390-utils. RHEL8 switched to versions from util-linux
> on s390x [4].
> 
> "phys_device" was added with sysfs support for memory hotplug in commit
> 3947be1969a9 ("[PATCH] memory hotplug: sysfs and add/remove functions")
> in 2005. It always returned 0.
> 
> s390x started returning something != 0 on some setups (if sclp.rzm is
> set by HW) in 2010 via commit 57b552ba0b2f("memory hotplug/s390: set
> phys_device").
> 
> For s390x, it allowed for identifying which memory block devices belong
> to the same memory increment (RZM). Only if all memory block devices
> comprising a single memory increment were offline, the memory could
> actually be removed in the hypervisor.
> 
> Since commit e5d709bb5fb7 ("s390/memory hotplug: provide
> memory_block_size_bytes() function") in 2013 a memory block devices
> spans at least one memory increment - which is why the interface isn't
> really helpful/used anymore (except by old lsmem/chmem tools).

Correct, so I do not see any problem for s390 with removing / changing
that for the upstream kernel. BTW, that commit also gave some relief
on the scaling issue, at least for s390. With increasing total memory
size, we also have increasing increment and thus memory block size.

Of course, that also has some limitations, IIRC max. 1 GB increment
size, but still better than the 256 MB default size.

> 
> There were once RFC patches to make use of it in ACPI, but it could be
> solved using different interfaces [1].
> 
> 
> While I'd love to rip it out completely, I think it would break old
> lsmem/chmem completely - and I assume that's not acceptable. I was
> wondering what would be considered safe to do now/in the future:
> 
> 1. Make it always return 0 (just as if "sclp.rzm" would be set to 0 on
> s390x). This will make old lsmem/chmem behave differently after
> switching to a new kernel, like if sclp.rzm would not be set by HW -
> AFAIU, it will assume all memory is in a single memory increment. Do we
> care?

No, at least not until that kernel change would be backported to some
old distribution level where we still use lsmem/chmem from s390-tools.
Given that this is just some clean-up w/o any functional benefit, and
hopefully w/o any negative impact, I think we can safely assume that no
distributor will do that "just for fun".

Even if there would be good reasons for backports, then I guess we also
have good reasons for backporting / switching to the util-linux version
of lsmem / chmem for such distribution levels. Alternatively, adjust the
s390-tools lsmem / chmem there.

But I would rather "rip it out completely" than just return 0. You'd
need some lsmem / chmem changes anyway, at least in case this would
ever be backported.

> 2. Restrict it to s390x only. It always returned 0 on other
> architectures, I was not able to find any user.
> 
> I think 2 should be safe to do (never used on other archs). I do wonder
> what the feelings are about 1.

Please don't add any s390-specific workarounds here, that does not
really sound like a clean-up, rather the opposite.

That being said, I do not really see the benefit of this change at
all. As Michal mentioned, there really should be some more fundamental
change. And from the rest of this thread, it also seems that phys_device
usage might not be the biggest issue here.

  parent reply	other threads:[~2020-09-22 13:56 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10 10:20 Ways to deprecate /sys/devices/system/memory/memoryX/phys_device ? David Hildenbrand
2020-09-10 20:00 ` Dave Hansen
2020-09-10 20:31   ` David Hildenbrand
2020-09-11  7:20     ` Michal Hocko
2020-09-11  8:09       ` David Hildenbrand
2020-09-11  9:12         ` Michal Hocko
2020-09-11 10:09           ` David Hildenbrand
2020-09-11 19:24             ` Dave Hansen
2020-09-11 19:35               ` Luck, Tony
2020-09-11 19:56                 ` David Hildenbrand
2020-09-11 20:09                   ` Luck, Tony
2020-09-11 20:49                     ` David Hildenbrand
2020-09-14 11:24             ` Michal Hocko
2020-09-14 12:14               ` David Hildenbrand
2020-09-10 20:57 ` Dave Hansen
2020-09-22 13:56 ` Gerald Schaefer [this message]
2020-09-25 14:49   ` David Hildenbrand
2020-09-25 15:00     ` Greg KH
2020-09-25 15:05       ` David Hildenbrand
2020-09-25 15:39     ` Michal Hocko
2020-09-25 15:47       ` David Hildenbrand

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=20200922155611.379373f7@thinkpad \
    --to=gerald.schaefer@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hca@linux.ibm.com \
    --cc=hoeppner@linux.ibm.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.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 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).