[PATCH v4 0/4] xentop: add physical CPU usage support

Jahan Murudi posted 4 patches 1 month, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250903102323.2553142-1-jahan.murudi.zg@renesas.com
tools/xentop/Makefile |   5 +-
tools/xentop/pcpu.c   | 141 ++++++++++++++++++++++++++++++++++++++++++
tools/xentop/pcpu.h   |  17 +++++
tools/xentop/xentop.c |  79 +++++++++++++++++------
4 files changed, 222 insertions(+), 20 deletions(-)
create mode 100644 tools/xentop/pcpu.c
create mode 100644 tools/xentop/pcpu.h
[PATCH v4 0/4] xentop: add physical CPU usage support
Posted by Jahan Murudi 1 month, 3 weeks ago
This is v4 of the patch series to add physical CPU monitoring to xentop.

Changes since v3:
-   Split the single large patch into a logical series of 3 smaller patches
    for easier review.
-   Fixed the memory allocation error handling in pcpu.c as pointed out by
    Anthony PERARD. The realloc() result is now assigned immediately to avoid
    invalid pointers on failure.
-   Simplified the time calculation by using a single global timestamp instead
    of a per-CPU array, as the time difference is the same for all cores.
-   Removed the unnecessary check for small time intervals which could lead to
    incorrect usage calculations.
-   Integrated the PCPU display with the existing print() function to ensure
    correct behavior in both interactive and batch modes.

The series adds a new '-p'/'--pcpus' flag to xentop. When enabled, it displays
a table showing the usage percentage of each physical CPU core in the system.

Jahan Murudi (4):
  xentop: add pcpu header and basic infrastructure
  xentop: add pcpu implementation with proper error handling
  xentop: update Makefile to link against libxenctrl
  xentop: integrate pcpu support into main program


 tools/xentop/Makefile |   5 +-
 tools/xentop/pcpu.c   | 141 ++++++++++++++++++++++++++++++++++++++++++
 tools/xentop/pcpu.h   |  17 +++++
 tools/xentop/xentop.c |  79 +++++++++++++++++------
 4 files changed, 222 insertions(+), 20 deletions(-)
 create mode 100644 tools/xentop/pcpu.c
 create mode 100644 tools/xentop/pcpu.h

-- 
2.34.1
Re: [PATCH v4 0/4] xentop: add physical CPU usage support
Posted by Anthony PERARD 1 month, 3 weeks ago
Hi Jahan,

On Wed, Sep 03, 2025 at 03:53:19PM +0530, Jahan Murudi wrote:
> This is v4 of the patch series to add physical CPU monitoring to xentop.
> 
> Changes since v3:
> -   Split the single large patch into a logical series of 3 smaller patches
>     for easier review.

The single patch in v3 was fine to review. It didn't really need to be
cut into several patches. Having one file change per patch is certainly
the worse possible way to cut one patch into several.

It might have been possible to separate into several patch in another
way, but it's a bit too late for that, there's already been several
reviews. What I like to do when I review a patch series, is to look at
the difference since the last review I gave, tools like
`git range-diff` and https://patchew.org/Xen/ can help with that.

Anyway, squashing back all the patch is the way to go I think.

I'll have a look at the changes.

-- 
Anthony PERARD
Re: [PATCH v4 0/4] xentop: add physical CPU usage support
Posted by Anthony PERARD 1 month, 3 weeks ago
On Wed, Sep 03, 2025 at 06:06:01PM +0200, Anthony PERARD wrote:
> Hi Jahan,
> 
> On Wed, Sep 03, 2025 at 03:53:19PM +0530, Jahan Murudi wrote:
> > This is v4 of the patch series to add physical CPU monitoring to xentop.
> > 
> > Changes since v3:
> > -   Split the single large patch into a logical series of 3 smaller patches
> >     for easier review.
> 
> The single patch in v3 was fine to review. It didn't really need to be
> cut into several patches. Having one file change per patch is certainly
> the worse possible way to cut one patch into several.
> 
> It might have been possible to separate into several patch in another
> way, but it's a bit too late for that, there's already been several
> reviews. What I like to do when I review a patch series, is to look at
> the difference since the last review I gave, tools like
> `git range-diff` and https://patchew.org/Xen/ can help with that.
> 
> Anyway, squashing back all the patch is the way to go I think.
> 
> I'll have a look at the changes.

So the code looks good, but only if it is a single patch like on v3.
Would you be fine if I merge the change, but squash all patch of v4
together and take the patch description from v3? (or the patch
description of the last patch with "integrate" replace by "introduce".)

If I do the squashing, I'll just ignore the remark to use "unsigned int"
instead of "int".

With all patch squash together, more or less: Reviewed-by: Anthony
PERARD <anthony.perard@vates.tech>

Cheers,

-- 
Anthony PERARD
RE: [PATCH v4 0/4] xentop: add physical CPU usage support
Posted by Jahan Murudi 1 month, 3 weeks ago
Hi Anthony,

>> Anyway, squashing back all the patch is the way to go I think.
>> 
>> I'll have a look at the changes.

> So the code looks good, but only if it is a single patch like on v3.
> Would you be fine if I merge the change, but squash all patch of v4 together and take the patch description from v3? (or the patch description of the last patch with "integrate" replace by "introduce".)

> If I do the squashing, I'll just ignore the remark to use "unsigned int"
> instead of "int".

> With all patch squash together, more or less: Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thank you for the review and feedback.
I completely understand. Please feel free to squash the v4 series back into a single patch for merging, using the patch description from v3 as you suggested.

Thanks again for your time and guidance.

Best regards,
Jahan