* (indicating pointers) should preferably be stuck onto the next token without intervening spaces everywhere, e.g.: const char * const *db_array => const char *const *db_array, because in a declaration the presence of the first * delimits the declarator list, which means that what is after the first * is specific to the variable being declared, and not to the next one in the list (that's IMO why that's the common practice for a single *).

• Remove remaining declaration for acpi_s4bios_sysctl() in acpi.c
• Add a comment in the acpi_sleep_machdep() functions explaining why we are just testing on S4 state before calling AcpiEnterSleepStateS4bios().

In D57391#1315144, @imp wrote:

I think that the specific need that drove the bell labs folks have aged out. It doesn't matter that much anymore. So I agree we're transitioning to spaces for new code, which this clearly is. I'm mostly agnostic, but I lean towards this is fine.

If there are no objections in a few days, I'll commit it.

(Update to the diff that was committed.)

(Closing by hand, forgot to tag the commit properly.)

In D57376#1314646, @markj wrote:

Is it possible to add a regression test?

Fine like this.

In D57305#1313259, @kib wrote:

So I do not think (...) generic vnode lifecycle there is to blame.

As detailed in an inline comment, this patch lowers the probability of the race, but does not make it disappear. The real problem is probably more along the lines of some code doing a vget() and assuming the obtained vnode is not doomed, which cannot be guaranteed in general.

Patch with suggested changes:

So, on multiple-package machines, this driver will malfunction as described in one of the inline comments (and I wasn't able to find a quick way to disable it in this case). I guess that's tolerable as a first shot.

Thanks for the explanations.

In D57264#1312294, @kib wrote:

Simplify greatly.

In D57232#1312240, @obiwac wrote:

@olce last point done by 7a11a6c64d4f52f41e63e535605d4187650c3651. Also updated sysctl descriptions in the acpi.4 manpage in 071d82d9973660742e771b23365a82eb8dd2dd0f

In D57258#1312247, @des wrote:

tbf I'm a bit annoyed at you in particular parachuting into this review.

Since wrmsr_early_safe_gp_handler() pops the hardware error code, isn't that going to botch upper frames of the stack for exceptions not pushing one?

In D57258#1312183, @des wrote:

Trying it out for yourself is free, Olivier.

Sorting values is great. Using Ta in the header item, and only there, however, looks more confusing than anything else. What are you trying to "fix" in these headers? If it's the usage of TABs, then all the tables should be converted, not just the headers.

Good to go (with one more small nit, the ordering of headers, to fix).

An update to the manual page would also be welcome, as its content is now essentially stale (except on the essential point that zzz serves to suspend :-)).

Maybe also change the comments in sys/compat/linuxkpi/common/src/linux_acpi.c:linux_handle_power_suspend_event() and sys/dev/acpi_support/acpi_ibm.c:acpi_ibm_eventhandler() accordingly?

This change looks OK per se, but before committing we have to confirm that we can get the same behavior as before. The divergence in behavior with acpiconf -s 3 should be fixed by D57239. Could you give it a try?