Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 15 Jul 2022 12:54:21 -0400
From:   Phil Auld <pauld@redhat.com>
To:     Nicolas Saenz Julienne <nsaenzju@redhat.com>
Cc:     linux-kernel@vger.kernel.org, fweisbec@gmail.com,
        bristot@redhat.com, vschneid@redhat.com, cmetcalf@ezchip.com,
        mgorman@suse.de, bsegall@google.com, rostedt@goodmis.org,
        dietmar.eggemann@arm.com, vincent.guittot@linaro.org,
        juri.lelli@redhat.com, peterz@infradead.org, mingo@redhat.com,
        mtosatti@redhat.com
Subject: Re: [PATCH] nohz/full, sched/rt: Fix missed tick-reenabling bug in
 dequeue_task_rt
Message-ID: <YtGbvaia8oOulMEo@lorien.usersys.redhat.com>
References: <20220628092259.330171-1-nsaenzju@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220628092259.330171-1-nsaenzju@redhat.com>
Precedence: bulk

On Tue, Jun 28, 2022 at 11:22:59AM +0200 Nicolas Saenz Julienne wrote:
> dequeue_task_rt() only decrements 'rt_rq->rt_nr_running' after having
> called sched_update_tick_dependency() preventing it from re-enabling the
> tick on systems that no longer have pending SCHED_RT tasks but have
> multiple runnable SCHED_OTHER tasks:
> 
>   dequeue_task_rt()
>     dequeue_rt_entity()
>       dequeue_rt_stack()
>         dequeue_top_rt_rq()
> 	  sub_nr_running()	// decrements rq->nr_running
> 	    sched_update_tick_dependency()
> 	      sched_can_stop_tick()	// checks rq->rt.rt_nr_running,
> 	      ...
>         __dequeue_rt_entity()
>           dec_rt_tasks()	// decrements rq->rt.rt_nr_running
> 	  ...
> 
> Every other scheduler class performs the operation in the opposite
> order, and sched_update_tick_dependency() expects the values to be
> updated as such. So avoid the misbehaviour by inverting the order in
> which the above operations are performed in the RT scheduler.
> 
> Fixes: 76d92ac305f2 ("sched: Migrate sched to use new tick dependency mask model")
> Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>

That's been around for a while. This change looks right and makes sense
to me.

Thanks,
Phil

Reviewed-by: Phil Auld <pauld@redhat.com>

--