Page MenuHomeFreeBSD

kernel: Add `%pV` to kernel's printf format string
Needs RevisionPublic

Authored by dumbbell on Fri, Jan 31, 2:31 PM.

Details

Reviewers
imp
Group Reviewers
linuxkpi
Summary

Why

The %pV conversion specification is used by linuxkpi and DRM drivers. This conversion expects a struct va_format argument. This struct has two field members:

  • fmt that points to another format string
  • va that points to a va_list.

It allows a "recursive" format string.

How

The format string parser checks if there is a V following a %p and recurse into kvprintf() with the struct va_format arguments if that's the case. It adds the return value to retval so that the length is correctly computed.

Otherwise it proceeds with the regular handling of %p.

This is part of the update of DRM drivers to Linux 6.7.

Sponsored by: The FreeBSD Foundation

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

It is a breaking change. I grepped freebsd-src and the only format strings that happen to have this %pV sequence are drivers that come from Linux and have a struct va_format argument (though this code may not be connected to the build).

This implementation follows the behavior of Linux and linuxkpi-based drivers don’t need any modifications. Another approach could be to swap the letters, e.g. %Vp to have V as a modifier of %p.

Yea, this is a stupid API that flies in the face of years and years of how printf works. The modifiers never follow the main format specifier.
It would break a driver than wnated to say "Memory used: %pV %pP" to signify Virtual and Physical addresses of something, though it's likely rare.
It would be better, imho, *NOT* to pollute the base FreeBSD with this junky API.
Instead, I'd create a wrapper for linux printf that re-writes format to at least convert pV to Vp and implement 'V' as a proper modifier. that would be much safer, less invasive and also allow us to have other, weird things from. Linux if we have to should some other format crazy happen in the future that's more likely to break things. Or better yet, just use %V directly... I'm quite worried doing this janky thing is going to prevent us from implementing something in the future and really don't want to see it. Though noticing it's for a var-args arg makes me even more hesitant.

Also, I'm a hard veto on the calling printf recursively, regardless of how/where we implement this. That's just dumb. It will cause all kinds of buffering problems and racing problems since we have to flush out the partial buffer now. I run into this all the time when I have multiple printfs in drivers that wind up interleaving when they aren't line by line. And the drm drivers are verbose enough this will be a problem.

imp requested changes to this revision.Fri, Jan 31, 2:46 PM
imp added inline comments.
sys/kern/subr_prf.c
841

This makes no sense to me... why would func being NULL mean we pass a different arg?

This revision now requires changes to proceed.Fri, Jan 31, 2:46 PM

If we start this ..

I also have an implementation for %pM (our %D) which is heavily used throughout the wireless drivers (I think we have 350-400 cases) out of which I only changed a few dominant ones.
I refrained from adding it so far.

In D48763#1112273, @imp wrote:

It would be better, imho, *NOT* to pollute the base FreeBSD with this junky API.

+1

Instead, I'd create a wrapper for linux printf that re-writes format to at least convert pV to Vp and implement 'V' as a proper modifier. that would be much safer, less invasive and also allow us to have other, weird things from. Linux if we have to should some other format crazy happen in the future that's more likely to break things.

It's a bit more tricky unfortunately if we want something which works on device_printf() etc. It'll need a "conversion intermediate" for the fmt string for quite a few functions and macros but that's definitively doable, just tedious but can also happen incrementally.

And there's the case (as I mentioned) where %pM on Linux only takes one argument but our %D takes two (which if rewriting I would probably convert to %#D and check for the sharpflag in subr_prf.c which should probably work w/o side effects) or we really take a HUGE bunch of modifiers for %p. So there may be ways but it'll never be 100% great and a bit dodgy.

In the past I was even considering expanding them entirely in LinuxKPI so subr_prf.c would never know about these modifiers or anything from Linux but va_* again makes this really hard.

So yes I am 100% for confining this in LinuxKPI for as much as we can if we find a good way -- especially if that way will work a bit more generic.