Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Add selftests for cpuidle_gov_ext

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/3/2025 8:38 AM, Alexei Starovoitov wrote:
[Some people who received this message don't often get email from alexei.starovoitov@xxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

On Mon, Sep 1, 2025 at 6:56 AM Lin Yikai <yikai.lin@xxxxxxxx> wrote:

+
+/*
+ * For some low-power scenarios,
+ * such as the screen off scenario of mobile devices
+ * (which will be determined by the user-space BPF program),
+ * we aim to choose a deeper state
+ * At this point, we will somewhat disregard the impact on CPU performance.
+ */
+int expect_deeper = 0;

...

+/* Select the next idle state */
+SEC("struct_ops.s/select")
+int BPF_PROG(bpf_cpuidle_select, struct cpuidle_driver *drv, struct cpuidle_device *dev)
+{
+       u32 key = 0;
+       s64 delta, latency_req, residency_ns;
+       int i;
+       unsigned long long disable;
+       struct cpuidle_gov_data *data;
+       struct cpuidle_state *cs;
+
+       data = bpf_map_lookup_percpu_elem(&cpuidle_gov_data_map, &key, dev->cpu);
+       if (!data) {
+               bpf_printk("cpuidle_gov_ext: [%s] cpuidle_gov_data_map is NULL\n", __func__);
+               return 0;
+       }
+       latency_req = bpf_cpuidle_ext_gov_latency_req(dev->cpu);
+       delta = bpf_tick_nohz_get_sleep_length();
+
+       update_predict_duration(data, drv, dev);
+       for (i = ARRAY_SIZE(drv->states)-1; i > 0; i--) {
+               if (i >= drv->state_count)
+                       continue;
+               cs = &drv->states[i];
+               disable = dev->states_usage[i].disable;
+               if (disable)
+                       continue;
+               if (latency_req < cs->exit_latency_ns)
+                       continue;
+
+               if (delta < cs->target_residency_ns)
+                       continue;
+
+               if (data->next_pred / FIT_FACTOR * ALPHA_SCALE < cs->target_residency_ns)
+                       continue;
+
+               break;
+       }
+       residency_ns = drv->states[i].target_residency_ns;
+       if (expect_deeper &&
+               i <= drv->state_count-2 &&
+               !dev->states_usage[i+1].disable &&
+               data->last_pred >= residency_ns &&
+               data->next_pred < residency_ns &&
+               data->next_pred / FIT_FACTOR * ALPHA_SCALE >= residency_ns &&
+               data->next_pred / FIT_FACTOR * ALPHA_SCALE >= data->last_duration &&
+               delta > residency_ns) {
+               i++;
+       }
+
+       return i;
+}

This function is the main programmability benefit that
you're claiming, right?

And user space knob 'expect_deeper' is the key difference
vs all existing governors ?

If so, I have to agree with Rafael. This doesn't look too compelling
to bolt bpf struct-ops onto cpuidle.

There must be a way to introduce user togglable knobs in the current
set of governors, no?

Other than that the patch set seems to be doing all the right things
from bpf perspective. KF_SLEEPABLE is missing in kfuncs and
the safety aspect needs to be thoroughly analyzed,
but before doing in-depth review the examples need to have more substance.
With real world benchmarks and results.
The commit log is saying:
"This implementation serves as a foundation, not a final solution"
It's understood that it's work-in-progress, but we need to see
more real usage before committing.

Thanks, Alexei, Song, and Rafael, for your valuable feedback.

So, I understand that the key requirement here is to demonstrate a real-world scenario example
that can be effectively used in production environments
and to provide benchmark results.

Next up, I'll focus on developing a real-world use case for mobile devices
and providing test results.
Thanks again for the helpful insights.






[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux