Re: [PATCH 0/2] sched: Refine scheduler naming for clarity and specificity

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

 



On Thu, 20 Feb 2025 02:20:18 +0800
Jemmy Wong <jemmywong512@xxxxxxxxx> wrote:

> Hi, I'm relatively new to Linux and eager to contribute to the community
> with some foundational work.

Welcome to the community.

> 
> I aim to help improve code readability and maintainability.
> While reading the scheduler code, I found some naming inconsistencies
> that made understanding the code more difficult.

I do plan on updating the scheduler comments soon, so that every function
has a purpose. But there's a lot of history to the scheduler code that you
really can't just go and rename items. It causes a lot of churn which also
causes noise in the git history, where git blame is used a lot to find why
things were done. Adding renames causes one more level of indirection that
makes it harder on the maintainers to do git forensics.

> 
> Specifically:
> 1. Some function names do not accurately reflect their purpose.
> 2. Certain names are too general, making it unclear what they represent.
> 3. Some concepts are ambiguous, leading to potential confusion.
> 
> - Rename ttwu_do_wakeup to ttwu_set_running:
>     - This function only sets task state to TASK_RUNNING,
>         not performing the actual wakeup.
> 
> - Rename update_cfs_group to update_cfs_group_shares:
>     - The name was too generic; specifying "shares" clarifies its purpose.
> 
> - Rename place_entity to update_entity_sched:
>     - The function does not handle entity placement but updates
>         sched info: vruntime, slice, and deadline.
> 
> - Rename update_load_<set, add, sub> to load_weight_<set, add, sub>:
>     - "load" can refer to either PELT load or load weight, causing ambiguity;
>         "load_weight" specifies it's dealing with weight.
> 
> - Rename struct sched_avg to struct sched_pelt:
>     - This structure includes not just average statistics
>         but also sums like <load, runnable, util>_sum, last_updae_time,
>         all of which are PELT (Per-Entity Load Tracking) metrics.
> 
> - Rename init_entity_runnable_average to init_entity_pelt
> 
> This patch aims to improve code readability and reduce confusion by
> ensuring names are more descriptive of their actual functionality or purpose.

I personally am not against these updates. But as I mentioned, there's a
lot of history here and it's really Peter Zijlstra's call (and he's been
against changes like this in the past).

So please do not be discourage if this doesn't get much traction.

-- Steve




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux