Date: Thu, 25 Sep 2014 12:09:31 -0600 From: Ian Lepore <ian@FreeBSD.org> To: Davide Italiano <davide@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: <1411668571.66615.247.camel@revolution.hippie.lan> In-Reply-To: <CACYV=-GMpMxEAs-X7umMdYX2Awf3G0La1cUGsXeH9MoX34CdxQ@mail.gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1411668571.66615.247.camel>