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;
IronPort-SDR: EhnRgSnZ7ptNGwLo7p31k09YQW4Oab9ACLKU+R6bF2YmLtocmLxanO4CB2fh+Pz2/nkZfDGff0
 NxEayFeJiuLfrAl4kax19XSvGmdLjgmPI4bg4ga/b5HEzeEnAlZ9v5vjz61B8OsBd8xhIkbCNC
 2BWwlxL6aFIMO2m3rHBrUHVn5wVylZctvUn3yEdaTvzXMG4Gm3rAGCXLDobztSJE9qg4LpDblH
 UiUbJcLpylKoZdbRfvdFOFiY0jUIMZ08+0+mgan6fvuYojfyY9kGZJj9/AZ7mgGJ6SnMmLgjyw
 l7I=
IronPort-SDR: DHcOYqEYCIexs8/1AcHZUJt/AVUVRRn3LG7DYarWurKjbUc5CRciV0zPkyHhdTbPq3hE0Yd2SI
 ySjs7UHV0sg/bXJxCHgfHIKpK/D0UqabGvs5lFNoclAyAE0Vx+n+21mAD2a6/OOg+WMDqttWPe
 tkyHpXZUeTX99jm3WTAZrLVuX4CcLSZ3V+Ia3lag7ubVyS0bIeigMYORyjoGHrs+t2BPvH7cMg
 YgPjtlGtf/I4tqzxqVBdGeApBWdYwSfzFsSpuFlTyRfULfwnf9urib6udEbZz80LmWt9L424mL
 e5c=
From:   "Schmid, Carsten" <Carsten_Schmid@mentor.com>
To:     Peter Zijlstra <peterz@infradead.org>
CC:     "mingo@redhat.com" <mingo@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "walken@google.com" <walken@google.com>,
        "dave@stgolabs.net" <dave@stgolabs.net>
Subject: AW: Crash in fair scheduler
Thread-Topic: Crash in fair scheduler
Thread-Index: AQHVqbfg64Wqju89WkedWbWijDxswKeoNgIAgAADvACAADdAgIAC7eEg
Date:   Thu, 5 Dec 2019 10:56:13 +0000
Message-ID: <ad7d2769ae3b4d8a88e4f67d5fb800cf@SVR-IES-MBX-03.mgc.mentorg.com>
References: <1575364273836.74450@mentor.com>
 <20191203103046.GJ2827@hirez.programming.kicks-ass.net>
 <656260cf50684c11a3122aca88dde0cb@SVR-IES-MBX-03.mgc.mentorg.com>
 <20191203140153.GP2844@hirez.programming.kicks-ass.net>
In-Reply-To: <20191203140153.GP2844@hirez.programming.kicks-ass.net>
Accept-Language: de-DE, en-IE, en-US
Content-Language: de-DE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> Von: Peter Zijlstra [mailto:peterz@infradead.org]

> 
> Exatly.
> 
> 
> I suppose one approach is to add code to both __enqueue_entity() and
> __dequeue_entity() that compares ->rb_leftmost to the result of
> rb_first(). That'd incur some overhead but it'd double check the logic.

As this is a ONCE without reproducer, i would prefer to use an approach
to exactly check for this case in the code path where it crashed.
Something like this (with pseudo-code):

simple:
....

do {
  se = pick_next_entity(..)
  if (unlikely(!se)) { /* here we check for the issue */
     write warning and some useful data to dmesg
     if (cur_rq->rb_leftmost == NULL) { /* our case */
       set cur_rq->rb_leftmost to itself as mentioned in the discussion
       se = pick_next_entity(..)       /* should now return a valid pointer */
     } else { /* another case happened, unknown */
        write warning to dmesg UNKNOWN
        panic() /* not known what to do here, would crash anyway. */
     }
  set_next_entity(se, ..)
  cfs_rq = group_cfs_rq(...)
} while (cfs_rq);

This will definitely not fix the rb_leftmost being NULL, but we can't tell
where this happened at all, so it's digging in the dark.
Maybe the data written to dmesg will help to diagnose further, if the issue
will happen again.
And, this will not affect performance much, as i have to take care of this too.

Thanks for all your suggestions.
Carsten