From: John Kacur <jkacur@redhat.com>
To: Tomas Glozar <tglozar@redhat.com>
Cc: linux-rt-users <linux-rt-users@vger.kernel.org>
Subject: Re: [PATCH 2/4] rteval: Minor improvements to CpuList class
Date: Fri, 15 Dec 2023 13:36:49 -0500 (EST) [thread overview]
Message-ID: <674c29f1-c3a8-9c2d-7c31-9cfb0961a321@redhat.com> (raw)
In-Reply-To: <CAP4=nvQ40NChnECdgLUJ7o4E_kwT3uTtfJ9kcxwEmuE_Dz5Hiw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1385 bytes --]
On Thu, 14 Dec 2023, Tomas Glozar wrote:
> út 12. 12. 2023 v 22:23 odesílatel John Kacur <jkacur@redhat.com> napsal:
> > Oops, your code is correct but you left in the old wrong explanation
> > you need to fix this up before I can apply this.
> >
>
> Yeah I completely missed that!
>
> > Are you sure you want to remove this? We have three states,
> > 1. cpu is online
> > 2. cpu is offline
> > 3. cpus is not legitimate, ie, we have 12 cpus and the code asks
> > whether cpu number 1000 is online
> > I think you're trying to write more elegant code than the original
> > implementation, but is it safer? Should we maybe check if the path
> > exists before calling sysread()?
> >
>
> What I didn't like about the original implementation was that checking
> if a CPU is online has little to do with the CpuList object. Thus, I
> made it static and removed the CpuList argument (self). I agree that
> the proper solution is to check if the CPU exists in the system
> (instead of in the cpulist), not removing the check altogether.
>
> Tomas
>
>
>
Alright, I amended the commit message to fix it.
I might make you check for the file's existence in some later patches
but I'm applying this for now to move forward, but I'm waiting for
any changes in patches 3 and 4 from you.
- Removed incorrect line from commit message
Signed-off-by: John Kacur <jkacur@redhat.com>
next prev parent reply other threads:[~2023-12-15 18:37 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-29 9:34 [PATCH 0/4] rteval: Add relative cpusets tglozar
2023-11-29 9:34 ` [PATCH 1/4] rteval: Refactor collapse_cpulist in systopology tglozar
2023-12-12 20:22 ` John Kacur
2023-11-29 9:34 ` [PATCH 2/4] rteval: Minor improvements to CpuList class tglozar
2023-12-12 21:23 ` John Kacur
2023-12-14 7:42 ` Tomas Glozar
2023-12-15 18:36 ` John Kacur [this message]
2023-11-29 9:34 ` [PATCH 3/4] rteval: Convert CpuList class to a module tglozar
2023-12-12 21:46 ` John Kacur
2023-12-14 8:08 ` Tomas Glozar
2023-12-14 8:28 ` Tomas Glozar
2023-12-14 15:05 ` John Kacur
2023-11-29 9:34 ` [PATCH 4/4] rteval: Add relative cpulists for measurements tglozar
2023-12-12 21:48 ` John Kacur
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=674c29f1-c3a8-9c2d-7c31-9cfb0961a321@redhat.com \
--to=jkacur@redhat.com \
--cc=linux-rt-users@vger.kernel.org \
--cc=tglozar@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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).