From: Dan Williams <dan.j.williams@intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
Keith Busch <keith.busch@intel.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
Linux Memory Management List <linux-mm@kvack.org>,
Dave Hansen <dave.hansen@intel.com>
Subject: Re: [PATCHv4 05/13] Documentation/ABI: Add new node sysfs attributes
Date: Sat, 19 Jan 2019 08:56:02 -0800 [thread overview]
Message-ID: <CAPcyv4jijnkW6E=0gpT3-qy5uOgTV-D7AN+CAu7mmdrRKGHvFg@mail.gmail.com> (raw)
In-Reply-To: <20190119090129.GC10836@kroah.com>
On Sat, Jan 19, 2019 at 1:01 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jan 18, 2019 at 01:08:02PM -0800, Dan Williams wrote:
> > On Thu, Jan 17, 2019 at 3:41 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Wed, Jan 16, 2019 at 6:59 PM Keith Busch <keith.busch@intel.com> wrote:
> > > >
> > > > Add entries for memory initiator and target node class attributes.
> > > >
> > > > Signed-off-by: Keith Busch <keith.busch@intel.com>
> > >
> > > I would recommend combining this with the previous patch, as the way
> > > it is now I need to look at two patches at the time. :-)
> > >
> > > > ---
> > > > Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
> > > > 1 file changed, 24 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/ABI/stable/sysfs-devices-node b/Documentation/ABI/stable/sysfs-devices-node
> > > > index 3e90e1f3bf0a..a9c47b4b0eee 100644
> > > > --- a/Documentation/ABI/stable/sysfs-devices-node
> > > > +++ b/Documentation/ABI/stable/sysfs-devices-node
> > > > @@ -90,4 +90,27 @@ Date: December 2009
> > > > Contact: Lee Schermerhorn <lee.schermerhorn@hp.com>
> > > > Description:
> > > > The node's huge page size control/query attributes.
> > > > - See Documentation/admin-guide/mm/hugetlbpage.rst
> > > > \ No newline at end of file
> > > > + See Documentation/admin-guide/mm/hugetlbpage.rst
> > > > +
> > > > +What: /sys/devices/system/node/nodeX/classY/
> > > > +Date: December 2018
> > > > +Contact: Keith Busch <keith.busch@intel.com>
> > > > +Description:
> > > > + The node's relationship to other nodes for access class "Y".
> > > > +
> > > > +What: /sys/devices/system/node/nodeX/classY/initiator_nodelist
> > > > +Date: December 2018
> > > > +Contact: Keith Busch <keith.busch@intel.com>
> > > > +Description:
> > > > + The node list of memory initiators that have class "Y" access
> > > > + to this node's memory. CPUs and other memory initiators in
> > > > + nodes not in the list accessing this node's memory may have
> > > > + different performance.
> > >
> > > This does not follow the general "one value per file" rule of sysfs (I
> > > know that there are other sysfs files with more than one value in
> > > them, but it is better to follow this rule as long as that makes
> > > sense).
> > >
> > > > +
> > > > +What: /sys/devices/system/node/nodeX/classY/target_nodelist
> > > > +Date: December 2018
> > > > +Contact: Keith Busch <keith.busch@intel.com>
> > > > +Description:
> > > > + The node list of memory targets that this initiator node has
> > > > + class "Y" access. Memory accesses from this node to nodes not
> > > > + in this list may have differet performance.
> > > > --
> > >
> > > Same here.
> > >
> > > And if you follow the recommendation given in the previous message
> > > (add "initiators" and "targets" subdirs under "classX"), you won't
> > > even need the two files above.
> >
> > This recommendation is in conflict with Greg's feedback about kobject
> > usage. If these are just "vanity" subdirs I think it's better to have
> > a multi-value sysfs file. This "list" style is already commonplace for
> > the /sys/devices/system hierarchy.
>
> If you do a subdirectory "correctly" (i.e. a name for an attribute
> group), that's fine. Just do not ever create a kobject just for a
> subdir, that will mess up userspace.
>
> And I hate the "multi-value" sysfs files, where at all possible, please
> do not copy past bad mistakes there. If you can make them individual
> files, please do that, it makes it easier to maintain and code the
> kernel for.
I agree in general about multi-value sysfs, but in this case we're
talking about a mask. Masks are a single value. That said I can get on
board with calling what 'cpulist' does a design mistake (human
readable mask), but otherwise switching to one file per item in the
mask is a mess for userspace to consume.
next prev parent reply other threads:[~2019-01-19 16:56 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-16 17:57 [PATCHv4 00/13] Heterogeneuos memory node attributes Keith Busch
2019-01-16 17:57 ` [PATCHv4 01/13] acpi: Create subtable parsing infrastructure Keith Busch
2019-01-16 17:57 ` [PATCHv4 02/13] acpi: Add HMAT to generic parsing tables Keith Busch
2019-01-16 17:57 ` [PATCHv4 03/13] acpi/hmat: Parse and report heterogeneous memory Keith Busch
2019-01-17 11:00 ` Rafael J. Wysocki
2019-01-16 17:57 ` [PATCHv4 04/13] node: Link memory nodes to their compute nodes Keith Busch
2019-01-17 11:26 ` Rafael J. Wysocki
2019-01-16 17:57 ` [PATCHv4 05/13] Documentation/ABI: Add new node sysfs attributes Keith Busch
2019-01-17 11:41 ` Rafael J. Wysocki
2019-01-18 20:42 ` Keith Busch
2019-01-18 21:08 ` Dan Williams
2019-01-19 9:01 ` Greg Kroah-Hartman
2019-01-19 16:56 ` Dan Williams [this message]
2019-01-20 16:19 ` Rafael J. Wysocki
2019-01-20 17:34 ` Dan Williams
2019-01-21 9:54 ` Rafael J. Wysocki
2019-01-20 16:16 ` Rafael J. Wysocki
2019-01-22 16:36 ` Keith Busch
2019-01-22 16:51 ` Rafael J. Wysocki
2019-01-22 16:54 ` Rafael J. Wysocki
2019-01-18 11:21 ` Jonathan Cameron
2019-01-18 16:35 ` Dan Williams
2019-01-16 17:57 ` [PATCHv4 06/13] acpi/hmat: Register processor domain to its memory Keith Busch
2019-01-17 12:11 ` Rafael J. Wysocki
2019-01-17 17:01 ` Dan Williams
2019-01-16 17:57 ` [PATCHv4 07/13] node: Add heterogenous memory access attributes Keith Busch
2019-01-17 15:03 ` Rafael J. Wysocki
2019-01-17 15:41 ` Greg Kroah-Hartman
2019-01-16 17:57 ` [PATCHv4 08/13] Documentation/ABI: Add node performance attributes Keith Busch
2019-01-17 15:09 ` Rafael J. Wysocki
2019-01-16 17:58 ` [PATCHv4 09/13] acpi/hmat: Register " Keith Busch
2019-01-17 15:21 ` Rafael J. Wysocki
2019-01-16 17:58 ` [PATCHv4 10/13] node: Add memory caching attributes Keith Busch
2019-01-17 16:00 ` Rafael J. Wysocki
2019-02-09 8:20 ` Brice Goglin
2019-02-10 17:19 ` Jonathan Cameron
2019-02-11 15:23 ` Keith Busch
2019-02-12 8:11 ` Brice Goglin
2019-02-12 8:49 ` Jonathan Cameron
2019-02-12 17:31 ` Keith Busch
2019-01-16 17:58 ` [PATCHv4 11/13] Documentation/ABI: Add node cache attributes Keith Busch
2019-01-17 16:25 ` Rafael J. Wysocki
2019-01-16 17:58 ` [PATCHv4 12/13] acpi/hmat: Register memory side " Keith Busch
2019-01-17 17:42 ` Rafael J. Wysocki
2019-01-16 17:58 ` [PATCHv4 13/13] doc/mm: New documentation for memory performance Keith Busch
2019-01-17 12:58 ` [PATCHv4 00/13] Heterogeneuos memory node attributes Balbir Singh
2019-01-17 15:44 ` Keith Busch
2019-01-18 13:16 ` Balbir Singh
2019-01-17 18:18 ` Jonathan Cameron
2019-01-17 19:47 ` Keith Busch
2019-01-18 11:12 ` Jonathan Cameron
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='CAPcyv4jijnkW6E=0gpT3-qy5uOgTV-D7AN+CAu7mmdrRKGHvFg@mail.gmail.com' \
--to=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;
as well as URLs for NNTP newsgroup(s).