Date: Sun, 5 Nov 2017 19:54:08 +0100 From: Oliver Pinter <oliver.pinter@hardenedbsd.org> To: Warner Losh <imp@bsdimp.com> Cc: "Conrad E. Meyer" <cem@freebsd.org>, Konstantin Belousov <kostikbel@gmail.com>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, src-committers <src-committers@freebsd.org> Subject: Re: svn commit: r325386 - head/sys/kern Message-ID: <CAPQ4ffu07Z8ZpGcGNk1dP4AN7J4FJM6T7ddn9OigVUM7X4Ot%2Bw@mail.gmail.com> In-Reply-To: <CANCZdfqApxd99G-HqVfe4x8EysYJVCAcHksqTeUuBAaYR1OjEA@mail.gmail.com> References: <201711041049.vA4AnZUE096709@repo.freebsd.org> <CAG6CVpUaXSX26Bc839kn96EXnUjtGyQ3_eNJPhsRB%2Bv6G6gH1Q@mail.gmail.com> <20171105130607.GA2566@kib.kiev.ua> <CAG6CVpV3SfV8VuNeMJEuN%2BMSu3424mK=HO_-YW9vRt9HEdcJZQ@mail.gmail.com> <20171105173032.GE2566@kib.kiev.ua> <CAG6CVpVQ4RSgz883ezsdExVDQ4RbSiR_y%2BZ5XWDNFeYBpMS-cQ@mail.gmail.com> <CANCZdfqApxd99G-HqVfe4x8EysYJVCAcHksqTeUuBAaYR1OjEA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11/5/17, Warner Losh <imp@bsdimp.com> wrote:
> On Sun, Nov 5, 2017 at 11:32 AM, Conrad Meyer <cem@freebsd.org> wrote:
>
>> E.g.,
>>
>> --- a/sys/ufs/ffs/ffs_alloc.c
>> +++ b/sys/ufs/ffs/ffs_alloc.c
>> @@ -304,8 +304,7 @@ retry:
>> }
>>
>> if (bp->b_blkno == bp->b_lblkno) {
>> - if (lbprev >= UFS_NDADDR)
>> - panic("ffs_realloccg: lbprev out of range");
>> + ASSERT(lbprev < UFS_NDADDR, "ffs_realloccg: lbprev out
>> of range");
>> bp->b_blkno = fsbtodb(fs, bprev);
>> }
>>
>
> Just a side point: All these should be programming errors.
Yes, they are programming errors, but the INVATIANTS and all of the
debugging kernel
facilities are disabled on -STABLE branches, and no one (except us)
running on system
with enabled debug stuffs.
So it would be nice to enable the debug facilities on -STABLE branches
and disable them
on -RELENG branch time. There was always several errors / patch, which
could be catch.
Now I don't want to search for them, but I uptreamed them one or two years ago.
> The bogus data
> that comes or could come from the FS itself should remain always-on panics.
> Well, actually, they should transition from always-on panics to some sort
> of degraded mount that would be more resilient in the face of such
> corruption. But failing that, they should remain always-on panics :)
>
> Warner
>
>
>
>> On Sun, Nov 5, 2017 at 9:30 AM, Konstantin Belousov <kostikbel@gmail.com>
>> wrote:
>> > On Sun, Nov 05, 2017 at 09:16:28AM -0800, Conrad Meyer wrote:
>> >> On Sun, Nov 5, 2017 at 5:06 AM, Konstantin Belousov <
>> kostikbel@gmail.com> wrote:
>> >> > On Sat, Nov 04, 2017 at 12:04:56PM -0700, Conrad Meyer wrote:
>> >> >> This is a functional change, because MPASS (via KASSERT) is only
>> >> >> enabled on DEBUG kernels. Ideally we would have a kind of ASSERT
>> that
>> >> >> worked on NODEBUG kernels.
>> >> > Why would we need such thing ?
>> >> >
>> >> > Our conventions are clear: consistency checks are normally done with
>> >> > KASSERT() and enabled for DEBUG (INVARIANTS or harder)
>> >> > configurations.
>> >> > We only leave explicit panics in the production kernels when there
>> >> > continuation of operations is worse then abort, e.g. when UFS
>> >> > detects
>> >> > the metadata corruption.
>> >>
>> >> An always-on assert construct would be precisely for the latter
>> >> scenario. Instead, we litter the tree with "if (!invariant) {
>> >> panic(); }."
>> > We do
>> >
>> > #ifdef INVARIANTS
>> > if (!condition) panic();
>> > #endif
>> >
>> > I do not understand what do you mean by 'instead'.
>>
>>
> _______________________________________________
> svn-src-head@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-head
> To unsubscribe, send any mail to "svn-src-head-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPQ4ffu07Z8ZpGcGNk1dP4AN7J4FJM6T7ddn9OigVUM7X4Ot%2Bw>