Re: [Xen-devel] [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline

[Marek Marczykowski <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Oct 31, 2018 at 03:33:58AM -0600, Jan Beulich wrote:
> >>> On 30.10.18 at 23:16, <marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> > --- a/tools/misc/xenpm.c
> > +++ b/tools/misc/xenpm.c
> > @@ -1231,7 +1231,7 @@ int main(int argc, char *argv[])
> >          xc_interface_close(xc_handle);
> >          return ret;
> >      }
> > -    max_cpu_nr = physinfo.nr_cpus;
> > +    max_cpu_nr = physinfo.max_cpu_id;
> 
> Isn't this off by 1 then? max_cpu_nr is misnamed, all loops using it
> are of the form
> 
>     for ( i = 0; i < max_cpu_nr; i++ )

Oh, you're right. This doesn't affect smt=off case, because the last cpu
is offline anyway...

> I'm also afraid there are further quirks here, with various constructs
> along the lines of (as bodies of aforementioned for())
> 
>         if ( show_cxstat_by_cpuid(xc_handle, i) == -ENODEV )
>             break;
> 
> which I suspect would terminate processing early when hitting a true
> gap (i.e. not one resulting from a parked CPU). 

How that would happen? Hotplug?
Also I've tried to look at error codes to distinguish offline cpu and
avoid printing that error, but looks EINVAL is used for a lot of cases.

> But I guess it wouldn't
> be appropriate to ask you to deal with this at the same time.
> 
> Jan
> 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel