KASSERT_WARN for asserting malloc(M_WAITOK) not in a non-sleepable thread

[Ian Lepore]

On Thu, 2014-09-25 at 11:16 -0700, Davide Italiano wrote:
> On Thu, Sep 25, 2014 at 11:09 AM, Ian Lepore <ian at freebsd.org> wrote:
> > On Thu, 2014-09-25 at 10:51 -0700, Davide Italiano wrote:
> >> On Thu, Sep 25, 2014 at 9:14 AM, Adrian Chadd <adrian at freebsd.org> wrote:
> >> > Hi,
> >> >
> >> > Please bring in KASSERT_WARN().
> >> >
> >> > I'm grown up enough to use KASSERT_WARN() along with handling the
> >> > invariant check myself in code. Having KASSERT_WARN() means I can add
> >> > in this rather than printf()s or device_printf()'s with various knobs
> >> > to remove it.
> >> >
> >> > (This is absolutely _not_ the "should KASSERT() optionally just log"
> >> > argument. I'm not going to get into that a second time.)
> >> >
> >> >
> >>
> >> If you put a KASSERT() inside your code -- probably you should be
> >> careful enough to put that iff you're sure that it should be always
> >> verified. No exceptions.
> >> People tend to be very lazy (including me). I don't expect everybody
> >> diligently upgrading KASSERT_WARN to KASSERT. So KASSERT_WARN start
> >> becoming more and more widespread, and people realize all of these
> >> need to be upgraded to KASSERT or removed. This generally happens
> >> after years. Yet. Another. Crusade.
> >> There's a lot of work in the kernel to remove old/wrong/naive  KPI
> >> from the kernel. jhb@ is looking at timeout()-> callout() conversion.
> >> I'm personally looking at dev_clone() removal. There are a lot of
> >> other examples.
> >> Adding KASSERT_WARN is a step backward, not a step forward, IMHO.
> >> That said, if you want to pollute the kernel, fine. I expressed my
> >> opinion, and I'm personally not happy about this, but I never stated
> >> I'm gonna stop you from doing that.
> >>
> >> Thanks,
> >>
> >> --
> >
> > IMO, this entire argument is ridiculous.  Some conditions are so insane
> > that you've got to stop immediately rather than make things worse.
> > Other conditions indicate problems, but the code can recover or
> > otherwise continue to operate safely.  Trying to define every possible
> > anomalous condition as either fatal or not worth mentioning is insane.
> >
> > Everyone is free to write code such as
> >
> > #ifdef INVARIANTS
> >   if (some_condition)
> >     printf("whatever warning\n");
> > #endif
> >
> > So let's be clear here:  the objections are to spelling that code
> > sequence KASSERT_WARN.  If you object, please explain what's wrong with
> > that spelling and how you would prefer it to be spelled.
> >
> > -- Ian
> >
> >
> 
> Take the assert out of the name. Call it DEBUG_WARN, or something else
> if you like.
> assert as a pretty *clear* and specific semantic, no need to mess
> around with it.
> 
> Thanks,
> 

To me, another "clear and specific semantic" that's associated with the
word 'assert' in C programming is that the test expression itself is
automatically printed as part of the diagnostic message.  That's not the
case in the FreeBSD kernel, so I guess we need to rename KASSERT as
well?

-- Ian