Page MenuHomeFreeBSD

Add KWARN facility to replace warn-only KASSERT
AbandonedPublic

Authored by cem on Apr 27 2016, 5:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 4, 3:16 PM
Unknown Object (File)
Feb 19 2024, 12:00 AM
Unknown Object (File)
Feb 5 2024, 7:31 AM
Unknown Object (File)
Jan 30 2024, 11:44 AM
Unknown Object (File)
Jan 14 2024, 1:31 AM
Unknown Object (File)
Jan 1 2024, 6:48 PM
Unknown Object (File)
Jan 1 2024, 6:29 AM
Unknown Object (File)
Dec 22 2023, 9:33 PM
Subscribers

Details

Reviewers
kan
jhb
Summary

A follow-up to https://reviews.freebsd.org/D6117 .

Suggested by: jhb

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3674
Build 3717: arc lint + arc unit

Event Timeline

cem retitled this revision from to Add KWARN facility to replace warn-only KASSERT.
cem updated this object.
cem edited the test plan for this revision. (Show Details)
cem added reviewers: uqs, jhb, kan.

Add missing headers and fix trivial compiler errors.

Include sys/types instead of cdefs for kdb.h

kan edited edge metadata.
This revision is now accepted and ready to land.Apr 27 2016, 9:10 PM
sys/kern/subr_kwarn.c
134

Using malloc() here isn't really a good idea IMO. The old kassert code didn't and just lived with the truncated string. I think that's fine. Using M_WAITOK prevents using KWARN() in things like ithreads or most of the places that WITNESS_WARN is used (and WITNESS_WARN is one of the things we could convert to KWARN as a use case).

That said, if we stop using malloc(), then the KTR bits aren't safe (and they were in the old kassert code) since ktr just stores the pointer, it doesn't snprintf it into a string. I would just axe the KTR bits entirely. If someone wishes they could insert a DTrace static probe instead perhaps to trace warnings.

sys/kern/subr_kwarn.c
134

The vast majority of the time the string will fit in 128 chars anyway. I'm okay with M_NOWAIT and truncating if that fails, for the ithread case.

WITNESS_WARN has its own system for aggregating witness warnings and logging or printing depending on configuration. I think it might be a step back to switch those to KWARN, lumping witness warnings in with everything else in the logs. But I'm not attached to it.

I would just axe the KTR bits entirely. If someone wishes they could insert a DTrace static probe instead perhaps to trace warnings.

Okay.

cem edited edge metadata.
  • Remove KTR
  • Use M_NOWAIT & truncate if allocation is unsuccessful
This revision now requires review to proceed.Apr 29 2016, 6:04 PM
cem marked an inline comment as done.Apr 29 2016, 6:04 PM

WITNESS_WARN is not the same as reversals. LORs use a separate mechanism that does mute duplicate warnings, but WITNESS_WARN always spews unconditionally. OTOH, the pps rate check wouldn't really work as it doesn't format a buffer to pass but calls printf directly, so perhaps it's not a good fit after all.

sys/kern/subr_kwarn.c
137

I'm still inclined to just drop the malloc() entirely. However, if you keep it there's no reason to run vsnprintf again. I would just do:

if (buf == NULL)
    buf =sbuf;
else {
    /* vsnprintf */
}

OTOH, the pps rate check wouldn't really work as it doesn't format a buffer to pass but calls printf directly, so perhaps it's not a good fit after all.

Can you elaborate? I don't understand the concern.

cem edited edge metadata.

Don't re-print if malloc failed.

cem marked an inline comment as done.Apr 29 2016, 9:06 PM

Add vkwarn(9) to use from witness_warn().