linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Anshuman Khandual <anshuman.khandual@arm.com>
To: Dave Hansen <dave.hansen@intel.com>,
	Keith Busch <keith.busch@intel.com>,
	linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-mm@kvack.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rafael Wysocki <rafael@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>
Subject: Re: [PATCH 0/7] ACPI HMAT memory sysfs representation
Date: Fri, 23 Nov 2018 12:12:56 +0530	[thread overview]
Message-ID: <ac942498-8966-6a9b-0e55-c79ae167c679@arm.com> (raw)
In-Reply-To: <f5315662-5c1a-68a3-4d04-21b4b5ca94b1@intel.com>



On 11/22/2018 11:31 PM, Dave Hansen wrote:
> On 11/22/18 3:52 AM, Anshuman Khandual wrote:
>>>
>>> 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.
> 
> My assumption has been that this kind of thing is too fancy for sysfs:

Applications need to know the matrix of multi attribute properties as
seen from various memory accessors/initiators to be able to bind them
to desired CPUs and memory. That gives applications true view of an
heterogeneous system. While I understand your concern here about the
sysfs (which can be worked around with probably multiple global files
may be if the size is a problem etc) but an insufficient interface is
definitely problematic in longer term. This is going to be an ABI which
is locked in for good. Hence even it might appear over engineering at
the moment but IMHO is the right thing to do.

> 
> Documentation/filesystems/sysfs.txt:
>> Attributes should be ASCII text files, preferably with only one value
>> per file. It is noted that it may not be efficient to contain only one
>> value per file, so it is socially acceptable to express an array of
>> values of the same type. 
>>
>> Mixing types, expressing multiple lines of data, and doing fancy
>> formatting of data is heavily frowned upon. Doing these things may get
>> you publicly humiliated and your code rewritten without notice. 
> 
> /proc/pid/pagemap is binary, not one-value-per-file and relatively
> complicated to parse.

I agree but it does provide user space really valuable information about
the faulted pages for it's VA space. Was there any better way of getting
it ? May be but at this point in time it is essential.

> 
> Do you really think following something like pagemap is the right model
> for sysfs.> 
> BTW, I'm not saying we don't need *some* interface like you propose.  We
> almost certainly will at some point.  I just don't think it will be in
> sysfs.

I am not saying doing this in sysfs is very elegant. I would rather have
a syscall read back (MAX_NODES * MAX_NODES * u64) attribute matrix from
the kernel. Probably a subset of that information can appear on sysfs to
speed of queries for various optimizations as Keith mentioned before. But
we will have to first evaluate and come to an agreement what constitutes
a comprehensive set for multi attribute properties. Are we willing to go
in the direction for inclusion of a new system call, subset of it appears
on sysfs etc ? My primary concern is not how the attribute information
appears on the sysfs but lack of it's completeness.

  reply	other threads:[~2018-11-23  6:43 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-14 22:49 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 [this message]
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
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=ac942498-8966-6a9b-0e55-c79ae167c679@arm.com \
    --to=anshuman.khandual@arm.com \
    --cc=dan.j.williams@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox