Received-SPF: pass (google.com: domain of linux-kernel+bounces-157360-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <99c1761a-9c00-4167-86e5-8a69cb6d9e5d@redhat.com>
Date: Wed, 24 Apr 2024 18:56:08 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] sched/fair: fix dlserver duplicate start and stop
To: "Vineeth Pillai (Google)" <vineeth@bitbyteword.org>,
 Daniel Bristot de Oliveira <bristot@kernel.org>,
 Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>,
 Juri Lelli <juri.lelli@redhat.com>,
 Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
 Steven Rostedt <rostedt@goodmis.org>, Ben Segall <bsegall@google.com>,
 Mel Gorman <mgorman@suse.de>, Valentin Schneider <vschneid@redhat.com>,
 linux-kernel@vger.kernel.org, Luca Abeni <luca.abeni@santannapisa.it>,
 Tommaso Cucinotta <tommaso.cucinotta@santannapisa.it>,
 Thomas Gleixner <tglx@linutronix.de>, Joel Fernandes
 <joel@joelfernandes.org>, Shuah Khan <skhan@linuxfoundation.org>,
 Phil Auld <pauld@redhat.com>, Suleiman Souhlal <suleiman@google.com>,
 Youssef Esmat <youssefesmat@google.com>
References: <20240412192304.3201847-1-vineeth@bitbyteword.org>
Content-Language: en-US, pt-BR, it-IT
From: Daniel Bristot de Oliveira <bristot@redhat.com>
In-Reply-To: <20240412192304.3201847-1-vineeth@bitbyteword.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 4/12/24 21:23, Vineeth Pillai (Google) wrote:
> dlserver is started when a cfs task is enqueued to an empty runqueue.
> Similarly dlserver is stopped when the last cfs task is dequeued. But
> this logic doesn't take care of the cfs throttling scenarios where the
> root cfs runqueue's h_nr_running stays zero while enqueue/dequeue can
> happen on throttled runqueues. This causes duplicate calls to start/stop
> and causes issues with deadline logic. One example is a WARN_ON in
> task_non_contending because of duplicate calls to dl_server_stop().
> 
> WARNING kernel: [ 1970.747755] ------------[ cut here ]------------
> WARNING kernel: [ 1970.747767] WARNING: CPU: 0 PID: 14202 at kernel/sched/deadline.c:352 task_non_contending+0x404/0x500
> 
> WARNING kernel: [ 1970.747868] CPU: 0 PID: 14202 Comm: tpm_manager_cli Tainted: G  W 5.15.152-22017-ga797d64dcd15 #1
> WARNING kernel: [ 1970.747874] Hardware name: HP Meep/Meep, BIOS Google_Meep.11297.250.0 01/25/2021
> WARNING kernel: [ 1970.747877] RIP: 0010:task_non_contending+0x404/0x500
> 
> WARNING kernel: [ 1970.747910] Call Trace:
> WARNING kernel: [ 1970.747914]  <TASK>
> WARNING kernel: [ 1970.747918]  ? __warn+0xa3/0x131
> WARNING kernel: [ 1970.747923]  ? task_non_contending+0x404/0x500
> WARNING kernel: [ 1970.747927]  ? report_bug+0x97/0xfa
> WARNING kernel: [ 1970.747932]  ? handle_bug+0x41/0x66
> WARNING kernel: [ 1970.747937]  ? exc_invalid_op+0x1b/0x4b
> WARNING kernel: [ 1970.747941]  ? asm_exc_invalid_op+0x16/0x20
> WARNING kernel: [ 1970.747946]  ? task_non_contending+0x404/0x500
> WARNING kernel: [ 1970.747949]  ? dequeue_dl_entity+0x112/0x2cd
> WARNING kernel: [ 1970.747952]  dl_server_stop+0x17/0x2b
> WARNING kernel: [ 1970.747956]  dequeue_task_fair+0x262/0x4c4
> WARNING kernel: [ 1970.747962]  __schedule+0x17c/0xf13
> WARNING kernel: [ 1970.747966]  ? update_load_avg+0x9b/0x611
> WARNING kernel: [ 1970.747970]  schedule+0x4e/0xd0
> WARNING kernel: [ 1970.747974]  schedule_hrtimeout_range_clock+0x10f/0x126
> WARNING kernel: [ 1970.747977]  ? add_wait_queue+0x4d/0x84
> WARNING kernel: [ 1970.747982]  poll_schedule_timeout+0x33/0x50
> WARNING kernel: [ 1970.747987]  do_sys_poll+0x4a3/0x626
> WARNING kernel: [ 1970.747993]  ? __se_sys_ppoll+0xdf/0xdf
> WARNING kernel: [ 1970.748000]  __se_sys_poll+0x70/0xf7
> WARNING kernel: [ 1970.748003]  do_syscall_64+0x51/0xa1
> WARNING kernel: [ 1970.748007]  entry_SYSCALL_64_after_hwframe+0x5c/0xc6
> WARNING kernel: [ 1970.748012] RIP: 0033:0x7bc815769510

I saw that as well.... but from a robot.. not with a reproducer.

> dlserver should be started on an idle root cfs rq, when
>  - enqueue on a non-throttled cfs_rq causing the root cfs rq to
>    go non-idle, or
>  - untthrottle results in the root cfs rq to go non-idle.
> 
> Similarly dlserver should be stopped on a non-idle root cfs rq, when
>  - dequeue on a non-throttled cfs_rq causing the root cfs rq to
>    go idle, or
>  - throttle results in the root cfs rq to go idle.


seem to make sense, I will add this check on v7.

Btw, as this is an ongoing thread discussion, instead of sending a patch, next time,
please reply to the patchset... it is easier for everybody to keep track.

-- Daniel