Date: Thu, 25 Sep 2014 11:16:10 -0700 From: Davide Italiano <davide@freebsd.org> To: Ian Lepore <ian@freebsd.org> Cc: Adrian Chadd <adrian@freebsd.org>, Bryan Drewery <bdrewery@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: KASSERT_WARN for asserting malloc(M_WAITOK) not in a non-sleepable thread Message-ID: <CACYV=-HDVxhQ=CDK7HWuK2t-rsFkW2gKD45NDjC86jQ2wq6v9A@mail.gmail.com> In-Reply-To: <1411668571.66615.247.camel@revolution.hippie.lan> References: <54236CD6.4050807@FreeBSD.org> <CACYV=-Eg69AQ72DOGppPSL7whJVCdcNg-auhBZ771iG7DfPdAw@mail.gmail.com> <5424392D.9030201@FreeBSD.org> <CAJ-Vmok5Xaa6aZvfL1GoW8C==dY47P=vKAEZhu16JhHjV%2BTk9g@mail.gmail.com> <CACYV=-GMpMxEAs-X7umMdYX2Awf3G0La1cUGsXeH9MoX34CdxQ@mail.gmail.com> <1411668571.66615.247.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Sep 25, 2014 at 11:09 AM, Ian Lepore <ian@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@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,
--
Davide
"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACYV=-HDVxhQ=CDK7HWuK2t-rsFkW2gKD45NDjC86jQ2wq6v9A>