Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190502194208.3535-1-daniel.vetter@ffwll.ch> <20190502194208.3535-2-daniel.vetter@ffwll.ch>
 <20190509092449.GA10828@jagdpanzerIV>
In-Reply-To: <20190509092449.GA10828@jagdpanzerIV>
From:   Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu, 9 May 2019 12:16:36 +0200
Message-ID: <CAKMK7uGXyvnvq4ZVN+ps8ua5vaT13a2h+sjep7PgKeVxZU=qug@mail.gmail.com>
Subject: Re: [PATCH 2/2] RFC: soft/hardlookup: taint kernel
To:     Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc:     Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Daniel Vetter <daniel.vetter@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Valdis Kletnieks <valdis.kletnieks@vt.edu>,
        Laurence Oberman <loberman@redhat.com>,
        Vincent Whitchurch <vincent.whitchurch@axis.com>,
        Don Zickus <dzickus@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Sinan Kaya <okaya@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, May 9, 2019 at 11:24 AM Sergey Senozhatsky
<sergey.senozhatsky.work@gmail.com> wrote:
>
> On (05/02/19 21:42), Daniel Vetter wrote:
> [..]
> > @@ -469,6 +469,8 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
> >               add_taint(TAINT_SOFTLOCKUP, LOCKDEP_STILL_OK);
> >               if (softlockup_panic)
> >                       panic("softlockup: hung tasks");
> > +             else
> > +                     add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
> >               __this_cpu_write(soft_watchdog_warn, true);
>
> Soft lockup sets TAINT_SOFTLOCKUP bit. Would it be enough for your CI?

I'm blind :-/ Yes this is totally useful.

> [..]
> > @@ -154,6 +154,8 @@ static void watchdog_overflow_callback(struct perf_event *event,
> >
> >               if (hardlockup_panic)
> >                       nmi_panic(regs, "Hard LOCKUP");
> > +             else
> > +                     add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
>
> Maybe you can mirror what soft lockup does. Add a HARDLOCKUP taint bit

We'd also want a taint for hung tasks (separate patch, same idea), not
sure it's a good idea to use a new taint bit for all of them? Atm we
don't check for all taint bits (some of them are set because of things
our testcases do, like module reload or setting unsafe kernel options
meant for testing only, so picking one of the bits we check already
was least resistance.

> +++ b/include/linux/kernel.h
> @@ -571,7 +571,8 @@ extern enum system_states {
>  #define TAINT_LIVEPATCH                        15
>  #define TAINT_AUX                      16
>  #define TAINT_RANDSTRUCT               17
> -#define TAINT_FLAGS_COUNT              18
> +#define TAINT_HARDLOCKUP               18
> +#define TAINT_FLAGS_COUNT              19
>
> and then set TAINT_HARDLOCKUP in watchdog_overflow_callback().
>
> Just a small idea, I'll leave this to more experienced people.

The hung_tasks taint wasn't all that positively received, I feels like
this will stay a hack private to our CI. Except if someone else pipes
up who wants this, then I'm happy to polish.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch