@kib Are you OK to be listed as a point of contact, or do your prefer it as is?

In D55118#1285935, @emaste wrote:

I've applied the recent update to my testing tree

• Restore the check on 21 statistics line, which was tailored for 24-line displays, and update the corresponding comment.
• Fix the commit message. After an additional check, no statistics line is actually missing. The fact that a new banner is printed *after* the statistics of the latest interval was really confusing and should be fixed. Additionally, we want to avoid a trailing banner in case of interruption or the requested number of statistics line has been reached.

Wait one interval to print the first statistics line.

In D44455#1284367, @koinec_yahoo.co.jp wrote:

Thank you for letting me know the results of discussion on the implementation policy. I understand the policy.
I will start working on the revisions now, but given my current situation, I think it will take several months. Please give me some time.

Don't leak the mountpoint's lock on VR_START_WRITE.

In D56062#1283586, @obiwac wrote:

Tried this out but devclass_get_count() gives 1 in the probe before anything was attached. i will find the time to investigate this soon

Looks good overall. Based on Microsoft's documentation, which leaves a lot to be desired, I would have called DSM_MODERN_TURN_ON_DISPLAY before DSM_MODERN_EXIT_NOTIF exactly as you're doing.

Looks good, but cannot be committed on its own as it would break powerd(8), see comment in D55606.

Some suggested changes, looks good.

Looks good.

In D56056#1282082, @obiwac wrote:

i think your stack is the wrong way around, right?

Looks good modulo jrtc27's suggestions.

Some additional tiny changes and this is good to go.

In D55819#1277553, @des wrote:

This, of course, assumes that neither the compiler nor the CPU reordered the writes...

Oh, and proposed commit's message herald line: "queue.h: Make STAILQ_INSERT_TAIL() maintain the tail invariant at all times".

In D55819#1277146, @jhb wrote:

In particular, is the invariant you are tripping over QCMD_STAILQ_CHECK_TAIL?

Some minor stuff to change.

Changes themselves look good, except for one mistake (wrong MSR).

Some small changes to do. Main point is to preserve sysctl_cppc_dump_handler()'s goal which is to always query the hardware (and thus never rely on cached values).

Fine with the idea and approach. Missing are MPASS() on setp in the CPU_WHICH_CPUSET and CPU_WHICH_JAIL cases though.

(Alternative: Don't compile this on 32-bit architectures at all, if if_iwx in reality does not support those.)

Looks good (some minor remarks in inline comments).

Overall looks good, but I have some remarks/questions given that that change now exposes the POWER_SSTATE_TRANSITION_* constants to userland.

1. I think we should consider renaming these constants, perhaps doing something as radical as POWER_SSTATE_TRANSITION_* => POWER_*, or perhaps keeping STATE (instead of SSTATE), before they are made publicly available (after which, we will have to provide them (almost) indefinitely).
2. Is this the granularity we want to expose? I'm OK with what is available right now, and we can later add more to that list in order to allow requesting *precise* states (i.e., not rely on the value of the power_*_stype variables to know which internal state we will finally go into). But just wanted to hear your thoughts.

In D55477#1272999, @gallatin wrote:

I hope you don't just mean having powerd influence the hardware control loop by tweaking the epp sysctl. That would lead to 2 control loops trying to govern the same thing, which often ends in chaos.

Idea is great. Currently, however, this revision depends on D55477 (which is rejected), so it needs to be re-based to become independent.

IOW, that would mean reopening PR 292615.

Mmm, this change from D55253 is deliberate. Autonomous mode does not work on some AMD CPUs, where setting "desired performance" to 0 actually causes lowest performance to be selected. As long as we do not have a user-understandable way to choose a better default, such as PM profile in ACPI's FADT, let's do like all other CPU freq/power drivers do (except hwpstate_intel(4)) and select maximum performance. Let's revise this only as part of actually implementing one.

As said previously, I'm opposed to this approach. So we have to discuss (for other people, this discussion has started through other channels as well, but I'll try to keep this public revision updated as much as possible).

Being out-of-time (and off for the next week), I only reviewed changes to ULE, and the first one in this revision. But I'll also take a look at the rest when coming back.