[PATCH v2 08/11] sched_ext: Add an event, BYPASS_DISPATCH

Changwoo Min posted 11 patches 1 year ago
There is a newer version of this series
[PATCH v2 08/11] sched_ext: Add an event, BYPASS_DISPATCH
Posted by Changwoo Min 1 year ago
Add a core event, BYPASS_DISPATCH, which represents how many tasks
have been dispatched in the bypass mode.

__scx_add_event() is used since the caller holds an rq lock,
so the preemption has already been disabled.

Signed-off-by: Changwoo Min <changwoo@igalia.com>
---
 kernel/sched/ext.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index bb363809e484..9a680774c1fc 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -1486,6 +1486,11 @@ struct scx_event_stats {
 	 */
 	u64		ENQ_SKIP_EXITING;
 
+	/*
+	 * The number of tasks dispatched in the bypassing mode.
+	 */
+	u64		BYPASS_DISPATCH;
+
 	/*
 	 * The number of times the bypassing mode has been activated.
 	 */
@@ -2941,6 +2946,8 @@ static int balance_one(struct rq *rq, struct task_struct *prev)
 	return false;
 
 has_tasks:
+	if (scx_rq_bypassing(rq))
+		__scx_add_event(BYPASS_DISPATCH, 1);
 	rq->scx.flags &= ~SCX_RQ_IN_BALANCE;
 	return true;
 }
@@ -5524,6 +5531,7 @@ static void scx_dump_state(struct scx_exit_info *ei, size_t dump_len)
 	scx_dump_event(s, &events, DISPATCH_LOCAL_DSQ_OFFLINE);
 	scx_dump_event(s, &events, DISPATCH_KEEP_LAST);
 	scx_dump_event(s, &events, ENQ_SKIP_EXITING);
+	scx_dump_event(s, &events, BYPASS_DISPATCH);
 	scx_dump_event(s, &events, BYPASS_ACTIVATE);
 
 	if (seq_buf_has_overflowed(&s) && dump_len >= sizeof(trunc_marker))
@@ -7856,6 +7864,7 @@ __bpf_kfunc void scx_bpf_event_stats(struct scx_event_stats *events,
 		scx_agg_event(&e_sys, e_cpu, SELECT_CPU_FALLBACK);
 		scx_agg_event(&e_sys, e_cpu, DISPATCH_LOCAL_DSQ_OFFLINE);
 		scx_agg_event(&e_sys, e_cpu, DISPATCH_KEEP_LAST);
+		scx_agg_event(&e_sys, e_cpu, BYPASS_DISPATCH);
 		scx_agg_event(&e_sys, e_cpu, BYPASS_ACTIVATE);
 	}
 
-- 
2.48.1
Re: [PATCH v2 08/11] sched_ext: Add an event, BYPASS_DISPATCH
Posted by Tejun Heo 1 year ago
On Sun, Jan 26, 2025 at 07:16:11PM +0900, Changwoo Min wrote:
...
>  has_tasks:
> +	if (scx_rq_bypassing(rq))
> +		__scx_add_event(BYPASS_DISPATCH, 1);

Can we do this at the sources even if that's a bit more code?

Thanks.

-- 
tejun
Re: [PATCH v2 08/11] sched_ext: Add an event, BYPASS_DISPATCH
Posted by Changwoo Min 1 year ago
Hello,

On 25. 1. 28. 05:05, Tejun Heo wrote:
> On Sun, Jan 26, 2025 at 07:16:11PM +0900, Changwoo Min wrote:
> ...
>>   has_tasks:
>> +	if (scx_rq_bypassing(rq))
>> +		__scx_add_event(BYPASS_DISPATCH, 1);
> 
> Can we do this at the sources even if that's a bit more code?

Sure. I will remove scx_add_event() and __scx_add_event() and will use 
this_cpu_add() and __this_cpu_add() directly.

Regards,
Changwoo Min
Re: [PATCH v2 08/11] sched_ext: Add an event, BYPASS_DISPATCH
Posted by Tejun Heo 1 year ago
On Thu, Jan 30, 2025 at 03:26:16PM +0900, Changwoo Min wrote:
> Hello,
> 
> On 25. 1. 28. 05:05, Tejun Heo wrote:
> > On Sun, Jan 26, 2025 at 07:16:11PM +0900, Changwoo Min wrote:
> > ...
> > >   has_tasks:
> > > +	if (scx_rq_bypassing(rq))
> > > +		__scx_add_event(BYPASS_DISPATCH, 1);
> > 
> > Can we do this at the sources even if that's a bit more code?
> 
> Sure. I will remove scx_add_event() and __scx_add_event() and will use
> this_cpu_add() and __this_cpu_add() directly.

Oh, that's not what I meant. I meant that in the code quoted above, the stat
is being incremented in a spot where most code paths converge and then the
specific stat condition is being tested again. It'd be better to update the
stat where the condition is detected initially.

Thanks.

-- 
tejun
Re: [PATCH v2 08/11] sched_ext: Add an event, BYPASS_DISPATCH
Posted by Changwoo Min 1 year ago
Hello,

On 25. 1. 30. 21:25, Tejun Heo wrote:
> On Thu, Jan 30, 2025 at 03:26:16PM +0900, Changwoo Min wrote:
>> Hello,
>>
>> On 25. 1. 28. 05:05, Tejun Heo wrote:
>>> On Sun, Jan 26, 2025 at 07:16:11PM +0900, Changwoo Min wrote:
>>> ...
>>>>    has_tasks:
>>>> +	if (scx_rq_bypassing(rq))
>>>> +		__scx_add_event(BYPASS_DISPATCH, 1);
>>>
>>> Can we do this at the sources even if that's a bit more code?
>>
>> Sure. I will remove scx_add_event() and __scx_add_event() and will use
>> this_cpu_add() and __this_cpu_add() directly.
> 
> Oh, that's not what I meant. I meant that in the code quoted above, the stat
> is being incremented in a spot where most code paths converge and then the
> specific stat condition is being tested again. It'd be better to update the
> stat where the condition is detected initially.

Aha, I got it. Thanks for the clarification.

Regards,
Changwoo Min