All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ed Tanous <ed@tanous.net>
To: Patrick Williams <patrick@stwcx.xyz>
Cc: "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
	"Ambrozewicz, Adrian" <adrian.ambrozewicz@linux.intel.com>
Subject: Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)
Date: Tue, 11 May 2021 09:26:41 -0700	[thread overview]
Message-ID: <CACWQX82QSD=1nZAYkP=CO=-ch_YcbRXmyvLt743F-hGspTNPqw@mail.gmail.com> (raw)
In-Reply-To: <YJqaKhKlZ7BZCGA9@heinlein>

On Tue, May 11, 2021 at 7:53 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Tue, Apr 27, 2021 at 01:52:51PM +0200, Ambrozewicz, Adrian wrote:
> > W dniu 4/9/2021 o 14:05, Patrick Williams pisze:
> > > On Wed, Apr 07, 2021 at 02:24:55PM +0200, Ambrozewicz, Adrian wrote:
> > >
> > > I suspect this would be the more difficult direction to go down.  There
> > > is already enough code that looks for sensors at specific paths that
> > > you'd have to track down and fix up.  Also, there has been some concern
> > > by some maintainers in other cases about having information in the paths
> > > have meaning and prefering to reduce the reliance on that.
> > >
> >
> > Please see message from Ed, as he's supposedly proposing to follow that
> > path. I don't have strong opinions on one or the other approach.
>
> I suspect you are not signing up to change all the existing code.  I'll
> look at Ed's reply though.
>
> > I've read the design, however one thing is not clear for me. My current
> > understanding was that for each association there would need to exist
> > some D-bus object at given path somewhere. Would i need my CPU inventory
> > service to also expose separate objects for each core for my association
> > to be 'legal', or could we represent some virtual hierarchy with no
> > actual D-Bus object in the system?
>
> Yes.  You would need an inventory object for each entity you want to
> attach sensors or metrics to.  This doesn't seem like it should really
> be an issue.  Other people have been working on adding CPU Cores already
> and there is the xyz.openbmc_project.Inventory.Item.CpuCore defined.
>
> > >> I've done some PoC implementation of ProcessorMetrics, which relies on
> > >> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or
> > >> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation
> > >> queries D-Bus for all sensors associated with given CPU and populates
> > >> properties if proper mapping was found.
> > >
> > > I'm not really grasping what the contents of this mapping property are.
> > > Generally we want to stay away from free-form strings having programatic
> > > uses.  Maybe if you can define these mappings as enumerations?
> > >
> > > What is the additional information you need besides the assocation from
> > > a sensor to its inventory item?
> >
> > In given example I would like my sensor to be source of information for
> > property defined by ProcessorMetrics schema. In the example I've used
> > property associated with given Core, thus CoreMetrics/12/UnhaltedCycles
> > maps directly to ProcessorMetrics sub-property. Enumerations could be
> > not enough as we have multiple informations to represent:
> > - association with given processor (done by ProcessorMetrics)
> > - association with given core (could that be handled by your proposed
> > design?)
> > - linking to given property
> >
> > Would the enumeration be used for the last element, while leaving
> > hierarchy problem to Associations?
>
> "UnhaltedCycles" is not a sensor, just to be clear.  IPMI might have
> called these kinds of things sensors but we do not.  Sensors for us
> measure physical properties.  This is just a property (or maybe a
> "metric") but it doesn't belong in the sensors namespace or modeled with
> a Sensor.Value.
>

This somewhat brings up a good point, what is a "sensor" on dbus?  I
would've assumed that these would be well represented as sensors, as
they do measure physical properties.  I hadn't assumed that they had
this limitation because we do have the
/xyz/openbmc_project/utilization namespace defined already.  If we're
going down the path of "must be physical" it would seem like
utilization should be moved out of the sensors interface?  Or am I
taking your statement too literally?

Not reusing sensor seems like it would lead to a lot of code
duplication, as every API would now need to understand everything
about every "publishes real time telemetry" type interface, and every
time we add a new one, we'd (probably) have to update the code to add
the new type.  That doesn't really seem maintainable to me for every
type of telemetry we might want;  If a sensor isn't the right place to
put it, how would we solve the "I want to publish all telemetry
values" type use cases?  Maybe namespace the interface itself, so we
can use the arg0namespace feature in match expressions?  I'm thinking
out loud at this point...

> I don't know why the "linking to a given property" would be a dbus
> representation.  Metric service should know which properties from dbus
> map to some metric entity, right?  For a one-user piece of information,
> I don't see a good reason to put this on dbus.

I think the issue here is how do you know that a specific value
relates to say, the processor utilization, or the ram utilization, or
the smart statistics?

>
> --
> Patrick Williams

  reply	other threads:[~2021-05-11 16:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07 12:24 Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas) Ambrozewicz, Adrian
2021-04-09 12:05 ` Patrick Williams
2021-04-27 11:52   ` Ambrozewicz, Adrian
2021-05-11 14:52     ` Patrick Williams
2021-05-11 16:26       ` Ed Tanous [this message]
2021-05-12 11:17         ` Ambrozewicz, Adrian
2021-05-17  7:38           ` Ambrozewicz, Adrian
2021-04-09 15:27 ` Ed Tanous
2021-04-27 11:38   ` Ambrozewicz, Adrian
2021-05-11 16:16     ` Ed Tanous

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='CACWQX82QSD=1nZAYkP=CO=-ch_YcbRXmyvLt743F-hGspTNPqw@mail.gmail.com' \
    --to=ed@tanous.net \
    --cc=adrian.ambrozewicz@linux.intel.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=patrick@stwcx.xyz \
    /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.