Re: Concerns with em.yaml YNL spec

[Changwoo Min]

Hi Donald,



Thanks for the feedback. I rearranged a paragraph in the original email
for easier reply.


On 12/12/25 00:54, Donald Hunter wrote:
> Hi,

> I guess the patch series was never cced to netdev or the YNL
> maintainers so this is my first opportunity to review it.
>

You are right. I think I ran get_maintainer.pl only before adding
em.yaml. That's my bad.

> I just spotted the new em.yaml YNL spec that got merged in
> bd26631ccdfd ("PM: EM: Add em.yaml and autogen files") as part of [1]
> because it introduced new yamllint reports:
>
> make -C tools/net/ynl/ lint
> make: Entering directory '/home/donaldh/net-next/tools/net/ynl'
> yamllint ../../../Documentation/netlink/specs
> ../../../Documentation/netlink/specs/em.yaml
>    3:1       warning  missing document start "---"  (document-start)
>    107:13    error    wrong indentation: expected 10 but found 12  (indentation)

I will fix these lint warnings. Besides fixing those warnings, it would
be useful to mention running lint somewhere. If there is a general
guideline for adding a new netlink YAML, I will revise it in a separate
patch.

> Other than the lint messages, there are a few concerns with the
> content of the spec:
>
> - pds, pd and ps might be meaningful to energy model experts but they
> are pretty meaningless to the rest of us. I see they are spelled out
> in the energy model header file so it would be better to use
> perf-domain, perf-table and perf-state here.

That makes sense. I will change as suggested.

> - I think the spec could have been called energy-model.yaml and the
> family called "energy-model" instead of "em".
>
> - the get-pds should probably be both do and dump which would give
> multi responses without the need for the pds attribute set (unless I'm
> missing something).
>

TODO


> - there are 2 flags attributes that are bare u64 which should have
> flags definitions in the YNL. Have a look at e.g. netdev.yaml to see
> examples of flags definitions.

Okay. I will add the following (from energy_model.h) for the flags:

#define EM_PERF_DOMAIN_MICROWATTS BIT(0)
#define EM_PERF_DOMAIN_SKIP_INEFFICIENCIES BIT(1)
#define EM_PERF_DOMAIN_ARTIFICIAL BIT(2)

> - the cpus attribute is a string which would appear to be a "%*pb"
> stringification of a bitmap. That's not very consumable for a UAPI and
> should probably use netlink bitmask or an array of cpu numbers or
> something.

Okay. I will change the string representation to an integer array of CPU
numbers.

> - there are no doc strings for any of the attributes. It would be
> great to do better for new YNL specs, esp. since there is better
> information in energy_model.h

Sure, I will add doc strings based on the comments in the
energy_model.h.

> Given that netlink is UAPI, I think we need to address these issues
> before v6.19 gets released.

Sure. I will prepare the changes quickly.

Regards,
Changwoo Min

> Thanks,
> Donald Hunter.
>
> [1] https://lore.kernel.org/all/20251020220914.320832-4-changwoo@xxxxxxxxxx/