Re: [PATCH v4 0/6] power: uncore power improvements and auto-detection

On 8/11/2026 12:21 AM, Stephen Hemminger wrote:
> On Wed, 29 Jul 2026 10:51:43 +0800
> Huisong Li <[email protected]> wrote:
>
>> This series improves the uncore power management in l3fwd-power
>> and adds automatic detection of uncore drivers in the power library.
>>
>> - Fix uncore deinitialization for non-legacy modes.
>> - Enable power QoS for all modes (not just legacy).
>> - Fix uncore help text and log messages.
>> - Relocate uncore initialization from arg parsing to init_power_library().
>> - Support automatic probing of uncore drivers in AUTO_DETECT and delete
>>    useless code for auto-detection uncore environment.
>>
>> ---
>>   v4:
>>   - Fix '!global_uncore_ops' to 'global_uncore_ops == NULL' from AI review.
>>   v3 link:
>>    https://inbox.dpdk.org/dev/[email protected]/
>>
>>   v3:
>>   - update feature to release_26_11.rst.
>>   - add a new patch to delte useless for automatic detection environment.
>>   v2 link:
>>    https://inbox.dpdk.org/dev/[email protected]/
>>
>>   v2:
>>   - Remove the patch which add global uncore init and deinit interface.
>>   - Remove the last patch in l3fwd-power about these new interface.
>>     Will send them after this series.
>>   v1 link:
>>    https://inbox.dpdk.org/dev/[email protected]/
>>
>> Huisong Li (6):
>>    examples/l3fwd-power: fix uncore deinit for non-legacy
>>    examples/l3fwd-power: enable power QoS for all modes
>>    examples/l3fwd-power: fix uncore help and log info
>>    examples/l3fwd-power: relocate uncore initialization
>>    power: support automatic detection of uncore driver
>>    power: remove unused auto-detection uncore
>>
>>   doc/guides/rel_notes/release_26_11.rst        |   6 +
>>   .../sample_app_ug/l3_forward_power_man.rst    |   2 +-
>>   examples/l3fwd-power/main.c                   | 256 +++++++++---------
>>   lib/power/rte_power_uncore.c                  |  78 +++---
>>   4 files changed, 185 insertions(+), 157 deletions(-)
>>
> Most of the developers are off enjoying summer vacation.
> But AI is still around and finds some things here:
> Note: some of what it complains about is noise.
Understand.
Thanks for AI review.
Will pick up some to fix.
>
> Applied to main (26.11-rc0) and reviewed against source. Applies cleanly.
>
> Note on check-git-log: it reports "Wrong 'Fixes' reference" for both
> tags. That is a shallow-clone artifact. Both references are correct
> against full history:
>    10db2a5b8724 ("examples/l3fwd-power: add options for uncore frequency")
>    3b3af56d3c9c ("power: fix uncore configuration")
>
>
> Patch 4/6 examples/l3fwd-power: relocate uncore initialization
>
> Warning: new file-scope variable g_uncore_cfg is neither static nor
> prefixed. It is used only in main.c. Make it static and drop the g_
> prefix, which is not DPDK style.
>
>    static struct uncore_cfg {
>            enum uncore_choice uncore_choice;
>            uint32_t freq_idx;
>    } uncore_cfg;
>
> Info: the out-of-range message carries over an off-by-one. The test
> rejects freq_idx > freq_array_len - 1 but the message says "choose a
> value from 0 to %d" with freq_array_len. Since the line is being
> rewritten anyway, print freq_array_len - 1.
>
>
> Patch 5/6 power: support automatic detection of uncore driver
>
> Error: resource leak in power_uncore_probe_driver().
>
>            ret = ops->init(0, 0);
>            if (ret == 0) {
>                    uint32_t env = power_uncore_driver_name2env(ops->name);
>                    if (env == UINT32_MAX)
>                            continue;
>                    ...
>                    ops->exit(0, 0);
>
> On the env == UINT32_MAX path the driver has already been initialized
> successfully but ops->exit(0, 0) is never called before moving to the
> next driver. For intel_uncore that leaves f_cur_min and f_cur_max open
> and the original min/max frequencies never written back. Restructure so
> exit() runs on every successful init:
>
>            ret = ops->init(0, 0);
>            if (ret != 0)
>                    continue;
>            env = power_uncore_driver_name2env(ops->name);
>            ops->exit(0, 0);
>            if (env == UINT32_MAX)
>                    continue;
>            global_uncore_env = env;
>            global_uncore_ops = ops;
>            break;
>
> Not reachable with the two in-tree drivers, since both "intel-uncore"
> and "amd-hsmp" are in uncore_env_str, but the branch is deliberate and
> is wrong as written.
>
> Error: function return type must be on its own line.
>
>    static uint32_t power_uncore_driver_name2env(char *name)
>    static int power_uncore_probe_driver(void)
>
> should be
>
>    static uint32_t
>    power_uncore_driver_name2env(const char *name)
>
>    static int
>    power_uncore_probe_driver(void)
>
> The parameter should also be const char *; ops->name is only read.
>
> Warning: the release note is filed under "New Features" but the commit
> carries Fixes: and Cc: [email protected]. Pick one. Either this is a
> backportable fix, in which case drop the New Features entry, or it is a
> feature, in which case drop Fixes and Cc: stable. As written the stable
> maintainers receive a patch that advertises itself as a new feature.
>
> Info: the AUTO_DETECT path now returns -ENODEV while every other error
> path in rte_power_set_uncore_env() returns -1. Not wrong, but the error
> convention is now inconsistent within one function.
>
>
> Patch 6/6 power: remove unused auto-detection uncore
>
> Warning: behaviour change to an exported stable symbol with no release
> note. Previously rte_power_uncore_init() fell back to auto-detection
> when no environment had been set; it now returns -1 with "Please set
> uncore environment first." Applications relying on the implicit
> fallback break at runtime with no build-time signal. Add a release note
> describing the new requirement.