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

On 9/3/2025 8:38 AM, Alexei Starovoitov wrote:

[Some people who received this message don't often get email from alexei.starovoitov@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

On Mon, Sep 1, 2025 at 6:56 AM Lin Yikai yikai.lin@vivo.com 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.