All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: anshuman.khandual@arm.com
Cc: Dave Hansen <dave.hansen@intel.com>,
	Keith Busch <keith.busch@intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux ACPI <linux-acpi@vger.kernel.org>,
	Linux MM <linux-mm@kvack.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH 0/7] ACPI HMAT memory sysfs representation
Date: Fri, 23 Nov 2018 09:15:45 -0800	[thread overview]
Message-ID: <CAPcyv4iP_+BhVD9NqYG_m8thXARbOfPMDpvg4hfxRATWom_MRQ@mail.gmail.com> (raw)
In-Reply-To: <0194f47c-d1d8-108e-a57f-0316adb9112b@arm.com>

On Thu, Nov 22, 2018 at 11:11 PM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 11/22/2018 11:38 PM, Dan Williams wrote:
> > On Thu, Nov 22, 2018 at 3:52 AM Anshuman Khandual
> > <anshuman.khandual@arm.com> wrote:
> >>
> >>
> >>
> >> On 11/19/2018 11:07 PM, Dave Hansen wrote:
> >>> On 11/18/18 9:44 PM, Anshuman Khandual wrote:
> >>>> IIUC NUMA re-work in principle involves these functional changes
> >>>>
> >>>> 1. Enumerating compute and memory nodes in heterogeneous environment (short/medium term)
> >>>
> >>> This patch set _does_ that, though.
> >>>
> >>>> 2. Enumerating memory node attributes as seen from the compute nodes (short/medium term)
> >>>
> >>> It does that as well (a subset at least).
> >>>
> >>> It sounds like the subset that's being exposed is insufficient for yo
> >>> We did that because we think doing anything but a subset in sysfs will
> >>> just blow up sysfs:  MAX_NUMNODES is as high as 1024, so if we have 4
> >>> attributes, that's at _least_ 1024*1024*4 files if we expose *all*
> >>> combinations.
> >> Each permutation need not be a separate file inside all possible NODE X
> >> (/sys/devices/system/node/nodeX) directories. It can be a top level file
> >> enumerating various attribute values for a given (X, Y) node pair based
> >> on an offset something like /proc/pid/pagemap.
> >>
> >>>
> >>> Do we agree that sysfs is unsuitable for exposing attributes in this manner?
> >>>
> >>
> >> Yes, for individual files. But this can be worked around with an offset
> >> based access from a top level global attributes file as mentioned above.
> >> Is there any particular advantage of using individual files for each
> >> given attribute ? I was wondering that a single unsigned long (u64) will
> >> be able to pack 8 different attributes where each individual attribute
> >> values can be abstracted out in 8 bits.
> >
> > sysfs has a 4K limit, and in general I don't think there is much
> > incremental value to go describe the entirety of the system from sysfs
> > or anywhere else in the kernel for that matter. It's simply too much> information to reasonably consume. Instead the kernel can describe the
>
> I agree that it may be some amount of information to parse but is crucial
> for any task on a heterogeneous system to evaluate (probably re-evaluate
> if the task moves around) its memory and CPU binding at runtime to make
> sure it has got the right one.

Can you provide some more evidence for this statement? It seems that
not many applications even care about basic numa let alone specific
memory targeting, at least according to libnumactl users.

     dnf repoquery --whatrequires numactl-libs

The kernel is the arbiter of memory, something is broken if
applications *need* to take on this responsibility. Yes, there will be
applications that want to tune and override the default kernel
behavior, but this is the exception, not the rule. The applications
that tend to care about specific memories also tend to be purpose
built for a given platform, and that lessens their reliance on the
kernel to enumerate all properties.

> > coarse boundaries and some semblance of "best" access initiator for a
> > given target. That should cover the "80%" case of what applications
>
> The current proposal just assumes that the best one is the nearest one.
> This may be true for bandwidth and latency but may not be true for some
> other properties. This assumptions should not be there while defining
> new ABI.

In fact, I tend to agree with you, but in my opinion that's an
argument to expose even less, not more. If we start with something
minimal that can be extended over time we lessen the risk of over
exposing details that don't matter in practice.

We're in the middle of a bit of a disaster with the VmFlags export in
/proc/$pid/smaps precisely because the implementation was too
comprehensive and applications started depending on details that the
kernel does not want to guarantee going forward. So there is a real
risk of being too descriptive in an interface design.

> > want to discover, for the other "20%" we likely need some userspace
> > library that can go parse these platform specific information sources
> > and supplement the kernel view. I also think a simpler kernel starting
> > point gives us room to go pull in more commonly used attributes if it
> > turns out they are useful, and avoid going down the path of exporting
> > attributes that have questionable value in practice.
> >
>
> Applications can just query platform information right now and just use
> them for mbind() without requiring this new interface.

No, they can't today, at least not for the topology details that HMAT
is describing. The platform-firmware to numa-node translation is
currently not complete. At a minimum we need a listing of initiator
ids and target ids. For an ACPI platform that is the proximity-domain
to numa-node-id translation information. Once that translation is in
place then a userspace library can consult the platform-specific
information sources to translate the platform-firmware view to the
Linux handles for those memories. Am I missing the library that does
this today?

> We are not even
> changing any core MM yet. So if it's just about identifying the node's
> memory properties it can be scanned from platform itself. But I agree
> we would like the kernel to start adding interfaces for multi attribute
> memory but all I am saying is that it has to be comprehensive. Some of
> the attributes have more usefulness now and some have less but the new
> ABI interface has to accommodate exporting all of these.

I get the sense we are talking past each other, can you give the next
level of detail on that "has to be comprehensive" statement?

  reply	other threads:[~2018-11-23 17:15 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-14 22:49 [PATCH 0/7] ACPI HMAT memory sysfs representation Keith Busch
2018-11-16  6:27 ` Anshuman Khandual
2018-11-16 15:51   ` Keith Busch
2018-11-19  1:52     ` Anshuman Khandual
2018-11-16 16:55   ` Dave Hansen
2018-11-19  5:44     ` Anshuman Khandual
2018-11-19 17:37       ` Dave Hansen
2018-11-22 11:52         ` Anshuman Khandual
2018-11-22 18:01           ` Dave Hansen
2018-11-23  6:42             ` Anshuman Khandual
2018-11-23 19:21               ` Dave Hansen
2018-11-23 21:13                 ` Dan Williams
2018-11-26 15:52                   ` Anshuman Khandual
2018-11-26 16:42                   ` Dave Hansen
2018-11-26 18:08                     ` Dan Williams
2018-11-27 10:15                       ` Anshuman Khandual
2018-11-27 16:56                         ` Dan Williams
2018-11-26 15:38                 ` Anshuman Khandual
2018-11-26 17:20                   ` Dave Hansen
2018-11-27  9:32                     ` Anshuman Khandual
2018-11-22 18:08           ` Dan Williams
2018-11-23  7:10             ` Anshuman Khandual
2018-11-23 17:15               ` Dan Williams [this message]
2018-11-27 14:05                 ` Anshuman Khandual

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=CAPcyv4iP_+BhVD9NqYG_m8thXARbOfPMDpvg4hfxRATWom_MRQ@mail.gmail.com \
    --to=dan.j.williams@intel.com \
    --cc=anshuman.khandual@arm.com \
    --cc=dave.hansen@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=keith.busch@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rafael@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 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.