From: Shuah Khan <skhan@linuxfoundation.org>
To: Ali Ahmet Memis <ali@iusegentoo.com>,
Shuah Khan <shuah@kernel.org>, Thomas Renninger <trenn@suse.com>,
"John B . Wyatt IV" <jwyatt@redhat.com>,
John Kacur <jkacur@redhat.com>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v2 0/2] cpupower: fix topology array handling
Date: Thu, 6 Aug 2026 09:50:03 -0600 [thread overview]
Message-ID: <22e2acc9-b822-4119-802a-74efc6df1509@linuxfoundation.org> (raw)
In-Reply-To: <20260805114305.97160-1-ali@iusegentoo.com>
On 8/5/26 05:43, Ali Ahmet Memis wrote:
> On Tue, 4 Aug 2026 14:45:48 -0600 Shuah Khan wrote:
>> Did you think about a scenario when the following check will be tru - i.e
>> core == -1 is trur?
>
> I went looking for one and could not find it, so that branch may well be
> dead. What I checked:
That is really the questions - the branch isn't dead, it is in the wrong
place.
Sounds like you don't have a real scenario to test this change. This why
I am not eager to take either of these patches.
However, did you consider simplifying the logic in these conditionals?
if(sysfs_topology_read_file(
cpu,
"physical_package_id",
&(cpu_top->core_info[cpu].pkg)) < 0) {
cpu_top->core_info[cpu].pkg = -1;
cpu_top->core_info[cpu].core = -1;
continue;
-- Is this continue necessary here?
}
if(sysfs_topology_read_file(
cpu,
"core_id",
&(cpu_top->core_info[cpu].core)) < 0) {
cpu_top->core_info[cpu].pkg = -1;
cpu_top->core_info[cpu].core = -1;
continue;
-- Is this continue necessary here?
}
I think the following logic makes sense without the continue(s)
if (cpu_top->core_info[cpu].core == -1) {
strncpy(cpu_top->core_info[cpu].core_cpu_list, "-1", CPULIST_BUFFER);
continue;
}
thanks,
-- Shuah
next prev parent reply other threads:[~2026-08-06 15:50 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-03 17:52 [PATCH v2 0/2] cpupower: fix topology array handling Ali Ahmet Memis
2026-08-03 17:52 ` [PATCH v2 1/2] cpupower: zero the topology array to avoid uninitialized reads Ali Ahmet Memis
2026-08-03 17:52 ` [PATCH v2 2/2] cpupower: do not count incomplete topology entries as physical cores Ali Ahmet Memis
2026-08-04 20:45 ` [PATCH v2 0/2] cpupower: fix topology array handling Shuah Khan
2026-08-05 11:43 ` Ali Ahmet Memis
2026-08-05 12:10 ` Ali Ahmet Memis
2026-08-06 15:50 ` Shuah Khan [this message]
2026-08-06 17:51 ` [PATCH v3 0/3] " Ali Ahmet Memis
2026-08-06 17:51 ` [PATCH v3 1/3] cpupower: zero the topology array to avoid uninitialized reads Ali Ahmet Memis
2026-08-06 17:51 ` [PATCH v3 2/3] cpupower: let the core == -1 check handle failed topology reads Ali Ahmet Memis
2026-08-06 17:51 ` [PATCH v3 3/3] cpupower: do not count incomplete topology entries as physical cores Ali Ahmet Memis
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=22e2acc9-b822-4119-802a-74efc6df1509@linuxfoundation.org \
--to=skhan@linuxfoundation.org \
--cc=ali@iusegentoo.com \
--cc=jkacur@redhat.com \
--cc=jwyatt@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=shuah@kernel.org \
--cc=trenn@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.