Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Message-ID: <585ca8ea-c371-dca4-52ef-0f2d2e019774@linux.dev>
Date:   Tue, 7 Nov 2023 11:49:15 +0800
MIME-Version: 1.0
From:   Yajun Deng <yajun.deng@linux.dev>
Subject: Re: [PATCH v3 1/3] sched: Don't account execution time for task group
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     mingo@redhat.com, juri.lelli@redhat.com,
        vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
        bristot@redhat.com, vschneid@redhat.com,
        linux-kernel@vger.kernel.org
References: <20231031125928.235685-1-yajun.deng@linux.dev>
 <20231031125928.235685-2-yajun.deng@linux.dev>
 <20231106124912.GM8262@noisy.programming.kicks-ass.net>
Content-Language: en-US
In-Reply-To: <20231106124912.GM8262@noisy.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2023/11/6 20:49, Peter Zijlstra wrote:
> On Tue, Oct 31, 2023 at 08:59:26PM +0800, Yajun Deng wrote:
>> The rt entity can be a task group. We will account execution time for
>> each task. For consistency, we don't need to account execution time for
>> task group.
>>
>> Pass a parameter to update_current_exec_runtime, let the caller decide
>> whether account execution time.
>>
>> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
>> index 6aaf0a3d6081..79cf80d73822 100644
>> --- a/kernel/sched/rt.c
>> +++ b/kernel/sched/rt.c
>> @@ -1018,7 +1018,8 @@ static void update_curr_rt(struct rq *rq)
>>   
>>   	trace_sched_stat_runtime(curr, delta_exec, 0);
>>   
>> -	update_current_exec_runtime(curr, now, delta_exec);
>> +	update_current_exec_runtime(curr, now, delta_exec,
>> +				    rt_entity_is_task(rt_se));
>>   
>>   	if (!rt_bandwidth_enabled())
>>   		return;
> ok, I think I've managed to confuse myself again.
>
> But at this point rt_se := &rq->curr->rt, which is *always* a task, no?


I think so, but it can be safer to use rt_entity_is_task().