[PATCH] sched_ext: Provides a sysfs 'events' to expose core event counters

[PATCH] sched_ext: Provides a sysfs 'events' to expose core event counters

[Changwoo Min]

Add a sysfs entry at /sys/kernel/sched_ext/events to expose core event
counters through the files system interface. Each line of the file shows
the event name and its counter value. In addition, the format of
scx_dump_event() is adjusted as the event name gets longer.

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

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index b2378e29f45a..987b88b2f0ed 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -1558,7 +1558,7 @@ static DEFINE_PER_CPU(struct scx_event_stats, event_stats_cpu);
  * @kind: a kind of event to dump
  */
 #define scx_dump_event(s, events, kind) do {					\
-	dump_line(&(s), "%30s: %16llu", #kind, (events)->kind);			\
+	dump_line(&(s), "%40s: %16llu", #kind, (events)->kind);			\
 } while (0)
 
 
@@ -4327,12 +4327,37 @@ static ssize_t scx_attr_enable_seq_show(struct kobject *kobj,
 }
 SCX_ATTR(enable_seq);
 
+#define scx_attr_event_show(buf, at, events, kind) ({				\
+	sysfs_emit_at(buf, at, "%40s: %16llu\n", #kind, (events)->kind);	\
+})
+
+static ssize_t scx_attr_events_show(struct kobject *kobj,
+				    struct kobj_attribute *ka, char *buf)
+{
+	struct scx_event_stats events;
+	int at = 0;
+
+	scx_bpf_events(&events, sizeof(events));
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_SELECT_CPU_FALLBACK);
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_DISPATCH_LOCAL_DSQ_OFFLINE);
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_DISPATCH_KEEP_LAST);
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_ENQ_SKIP_EXITING);
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_ENQ_SKIP_MIGRATION_DISABLED);
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_ENQ_SLICE_DFL);
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_BYPASS_DURATION);
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_BYPASS_DISPATCH);
+	at += scx_attr_event_show(buf, at, &events, SCX_EV_BYPASS_ACTIVATE);
+	return at;
+}
+SCX_ATTR(events);
+
 static struct attribute *scx_global_attrs[] = {
 	&scx_attr_state.attr,
 	&scx_attr_switch_all.attr,
 	&scx_attr_nr_rejected.attr,
 	&scx_attr_hotplug_seq.attr,
 	&scx_attr_enable_seq.attr,
+	&scx_attr_events.attr,
 	NULL,
 };
 
-- 
2.48.1

Re: [PATCH] sched_ext: Provides a sysfs 'events' to expose core event counters

[Tejun Heo]

Hello,

On Mon, Feb 10, 2025 at 11:36:43PM +0900, Changwoo Min wrote:
...
> +#define scx_attr_event_show(buf, at, events, kind) ({				\
> +	sysfs_emit_at(buf, at, "%40s: %16llu\n", #kind, (events)->kind);	\
> +})

It's nice to format things in tabular forms but things under /sys lean more
towards simpler formatting, so maybe just do "%s %16llu\n"?

>  static struct attribute *scx_global_attrs[] = {
>  	&scx_attr_state.attr,
>  	&scx_attr_switch_all.attr,
>  	&scx_attr_nr_rejected.attr,
>  	&scx_attr_hotplug_seq.attr,
>  	&scx_attr_enable_seq.attr,
> +	&scx_attr_events.attr,

This probably should belong to the root/ subdir as we'd probably want to
keep the event counter separate per scheduler instance in the
multi-scheduler future.

Thanks.

-- 
tejun

Re: [PATCH] sched_ext: Provides a sysfs 'events' to expose core event counters

[Changwoo Min]

Hello,

Thank you for the review!

On 25. 2. 11. 02:28, Tejun Heo wrote:
> Hello,
> 
> On Mon, Feb 10, 2025 at 11:36:43PM +0900, Changwoo Min wrote:
> ...
>> +#define scx_attr_event_show(buf, at, events, kind) ({				\
>> +	sysfs_emit_at(buf, at, "%40s: %16llu\n", #kind, (events)->kind);	\
>> +})
> 
> It's nice to format things in tabular forms but things under /sys lean more
> towards simpler formatting, so maybe just do "%s %16llu\n"?

Sure, I will change it to the simplest form, "%s %llu\n".

> 
>>   static struct attribute *scx_global_attrs[] = {
>>   	&scx_attr_state.attr,
>>   	&scx_attr_switch_all.attr,
>>   	&scx_attr_nr_rejected.attr,
>>   	&scx_attr_hotplug_seq.attr,
>>   	&scx_attr_enable_seq.attr,
>> +	&scx_attr_events.attr,
> 
> This probably should belong to the root/ subdir as we'd probably want to
> keep the event counter separate per scheduler instance in the
> multi-scheduler future.

I feel this is a bit contradictory to the need to access the core
event counters even after an scx scheduler is unloaded. In the
current implementation, root/ subdir appears and disappears when
an scx scheduler is loaded and unloaded.

We may change the scx_ktype to something similar to
scx_global_attr_group in order to keep root/ subdir. We then show
an empty file for root/ops when no scx scheduler is loaded while
keep the root/events file intact. I am not sure if this is what
we want.

What do you think?

Regards,
Changwoo Min

Re: [PATCH] sched_ext: Provides a sysfs 'events' to expose core event counters

[Tejun Heo]

Hello, sorry about the late reply.

On Tue, Feb 11, 2025 at 09:57:08AM +0900, Changwoo Min wrote:
> > This probably should belong to the root/ subdir as we'd probably want to
> > keep the event counter separate per scheduler instance in the
> > multi-scheduler future.
> 
> I feel this is a bit contradictory to the need to access the core
> event counters even after an scx scheduler is unloaded. In the
> current implementation, root/ subdir appears and disappears when
> an scx scheduler is loaded and unloaded.
> 
> We may change the scx_ktype to something similar to
> scx_global_attr_group in order to keep root/ subdir. We then show
> an empty file for root/ops when no scx scheduler is loaded while
> keep the root/events file intact. I am not sure if this is what
> we want.
> 
> What do you think?

Hmm... I don't think we can keep the directory for counters of schedulers
that have been unloaded. Looks like the right thing to do is giving up on
the idea of being able to access the counters after the scheduler is
unloaded. The counters are dumped on error exits, so hopefully this isn't
too big a loss. What do you think?

Thanks.

-- 
tejun

Re: [PATCH] sched_ext: Provides a sysfs 'events' to expose core event counters

[Changwoo Min]

Hello Tejun,


Thank you for the review!

On 25. 2. 14. 01:45, Tejun Heo wrote:
> Hello, sorry about the late reply.
> 
> On Tue, Feb 11, 2025 at 09:57:08AM +0900, Changwoo Min wrote:
>>> This probably should belong to the root/ subdir as we'd probably want to
>>> keep the event counter separate per scheduler instance in the
>>> multi-scheduler future.
>>
>> I feel this is a bit contradictory to the need to access the core
>> event counters even after an scx scheduler is unloaded. In the
>> current implementation, root/ subdir appears and disappears when
>> an scx scheduler is loaded and unloaded.
>>
>> We may change the scx_ktype to something similar to
>> scx_global_attr_group in order to keep root/ subdir. We then show
>> an empty file for root/ops when no scx scheduler is loaded while
>> keep the root/events file intact. I am not sure if this is what
>> we want.
>>
>> What do you think?
> 
> Hmm... I don't think we can keep the directory for counters of schedulers
> that have been unloaded. Looks like the right thing to do is giving up on
> the idea of being able to access the counters after the scheduler is
> unloaded. The counters are dumped on error exits, so hopefully this isn't
> too big a loss. What do you think?

Yes, dumping the event counters is a part of scx_dump_state().
I agree. That's a reasonable choice. I will move 'events' under
the root/ subdir and send out a new version.

Regards,
Changwoo Min