Linux Trace Kernel
 help / color / mirror / Atom feed
From: Jemmy Wong <jemmywong512@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Jemmy <jemmywong512@gmail.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Jonathan Corbet <corbet@lwn.net>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH 0/2] sched: Refine scheduler naming for clarity and specificity
Date: Thu, 20 Feb 2025 14:48:41 +0800	[thread overview]
Message-ID: <EBFD5AF7-8B9D-480E-9A01-74898E4EBC60@gmail.com> (raw)
In-Reply-To: <20250219172443.5a78401d@gandalf.local.home>

Thanks for filling me in on the background of the code status
and the potential consequences of these changes. 
I really appreciate the context! 

As Phil Karlton once said:
There are only two hard things in Computer Science: 
cache invalidation and naming things. 

A good name makes code easier to read and understand.
I personally value code readability and maintainability. 
However, based on your input and the community’s opinion,
I agree that these changes could introduce indirection 
for git blame and make maintenance even harder. 

So, I’ll discard the changes.

BR,
Jemmy  

> On Feb 20, 2025, at 6:24 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Thu, 20 Feb 2025 02:20:18 +0800
> Jemmy Wong <jemmywong512@gmail.com> 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



      reply	other threads:[~2025-02-20  6:49 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-19 18:20 [PATCH 0/2] sched: Refine scheduler naming for clarity and specificity Jemmy Wong
2025-02-19 18:20 ` [PATCH 1/2] " Jemmy Wong
2025-02-19 18:42   ` Steven Rostedt
2025-02-19 19:04     ` Jemmy Wong
2025-02-19 19:10       ` Steven Rostedt
2025-02-19 18:20 ` [PATCH 2/2] sched: Refine sched_avg " Jemmy Wong
2025-02-19 22:24 ` [PATCH 0/2] sched: Refine scheduler " Steven Rostedt
2025-02-20  6:48   ` Jemmy Wong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=EBFD5AF7-8B9D-480E-9A01-74898E4EBC60@gmail.com \
    --to=jemmywong512@gmail.com \
    --cc=bsegall@google.com \
    --cc=corbet@lwn.net \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mgorman@suse.de \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox