Messages in this thread | | | From | "Brown, Len" <> | Subject | RE: [PATCH 2/2] turbostat, add set_base_cpu() | Date | Tue, 26 May 2015 00:32:39 +0000 |
| |
> -----Original Message----- > From: Prarit Bhargava [mailto:prarit@redhat.com] > Sent: Friday, May 22, 2015 6:30 PM > To: Brown, Len > Cc: linux-kernel@vger.kernel.org; linux-pm@vger.kernel.org; Semin, Andrey > Subject: Re: [PATCH 2/2] turbostat, add set_base_cpu() > > > > On 05/22/2015 11:55 AM, Brown, Len wrote: > >> +void set_base_cpu(void) > >> +{ > >> + int cpu; > >> + > >> + for (cpu = 0; cpu <= topo.max_cpu_num; ++cpu) { > >> + if (cpu_is_not_present(cpu)) > >> + continue; > >> + base_cpu = cpu; > >> + break; > >> + } > >> + > >> + if (base_cpu == -1) > >> + err(-ENODEV, "No valid cpus found"); > >> +} > > > > > > cpu0 hard-coding is indeed arbitrary. > > However, so is this proposed replacement, base_cpu. > > Either may not match where turbostat is currently running, > > and thus could provoke unnecessary cross-calls to get there. > > > > I think it would be better to ask getcpu(2) where we are already > running, > > and simply use that one. I think we can call it once and cache it, > > as you proposed, rather than multiple system calls. > > Any objection to sched_getcpu()? That way the code is simply > > base_cpu = sched_getcpu(); > > if (base_cpu == -1) > err(-ENODEV, "No valid cpus found");
Agreed, that is better than invoking the syscall directly, as we already are using the sched.h interface in this code.
thanks, -Len
| |