[PATCH] x86/domain: Dump domain paging pool sizes with the rest of the pageframe info

Aidan Allen posted 1 patch 3 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/CANgacT8YrH66iUEPr60AVytXYMivPzxC0SO4._5FRPohjk1Vo2w8Q@mail.gmail.com
xen/arch/x86/domain.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] x86/domain: Dump domain paging pool sizes with the rest of the pageframe info
Posted by Aidan Allen 3 months, 1 week ago
Added the ability to view the paging pool size via the debug keys

Signed-off-by: Aidan Allen <aidan.allen1@cloud.com>
---
 xen/arch/x86/domain.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 503a55705b..b64d09a9c8 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -253,6 +253,10 @@ void dump_pageframe_info(struct domain *d)
                page->count_info, page->u.inuse.type_info);
     }

+    printk("    Domain paging pool: total: %d, free: %d, p2m: %d\n",
+           d->arch.paging.total_pages, d->arch.paging.free_pages,
+           d->arch.paging.p2m_pages);
+
     nrspin_unlock(&d->page_alloc_lock);
 }

--
2.47.1
Re: [PATCH] x86/domain: Dump domain paging pool sizes with the rest of the pageframe info
Posted by Jan Beulich 3 months, 1 week ago
On 24.07.2025 13:40, Aidan Allen wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -253,6 +253,10 @@ void dump_pageframe_info(struct domain *d)
>                 page->count_info, page->u.inuse.type_info);
>      }
> 
> +    printk("    Domain paging pool: total: %d, free: %d, p2m: %d\n",
> +           d->arch.paging.total_pages, d->arch.paging.free_pages,
> +           d->arch.paging.p2m_pages);

Should this perhaps be gated, to avoid printing all zeroes for domains not
using an paging mode? Whether to use paging_mode_enabled() to do so I'm not
sure, as a domain with no paging mode enabled could still have a paging
pool configured.

Jan
Re: [PATCH] x86/domain: Dump domain paging pool sizes with the rest of the pageframe info
Posted by Aidan Allen 3 months, 1 week ago
On Thu, Jul 24, 2025 at 12:52 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 24.07.2025 13:40, Aidan Allen wrote:
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -253,6 +253,10 @@ void dump_pageframe_info(struct domain *d)
> >                 page->count_info, page->u.inuse.type_info);
> >      }
> >
> > +    printk("    Domain paging pool: total: %d, free: %d, p2m: %d\n",
> > +           d->arch.paging.total_pages, d->arch.paging.free_pages,
> > +           d->arch.paging.p2m_pages);
>
> Should this perhaps be gated, to avoid printing all zeroes for domains not
> using an paging mode? Whether to use paging_mode_enabled() to do so I'm not
> sure, as a domain with no paging mode enabled could still have a paging
> pool configured.
>

Hi Jan,
I can gate this on total_pages > 0 if that would address your concern
with paging_mode_enabled().

However, regarding the gating itself, I don't believe that the line
saved by not printing this
is worth the potential confusion of somebody looking for this
information and not finding
it, I would prefer to leave it printing all zeroes.

Best, Aidan.
Re: [PATCH] x86/domain: Dump domain paging pool sizes with the rest of the pageframe info
Posted by Jan Beulich 3 months, 1 week ago
On 24.07.2025 17:59, Aidan Allen wrote:
> On Thu, Jul 24, 2025 at 12:52 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 24.07.2025 13:40, Aidan Allen wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -253,6 +253,10 @@ void dump_pageframe_info(struct domain *d)
>>>                 page->count_info, page->u.inuse.type_info);
>>>      }
>>>
>>> +    printk("    Domain paging pool: total: %d, free: %d, p2m: %d\n",
>>> +           d->arch.paging.total_pages, d->arch.paging.free_pages,
>>> +           d->arch.paging.p2m_pages);
>>
>> Should this perhaps be gated, to avoid printing all zeroes for domains not
>> using an paging mode? Whether to use paging_mode_enabled() to do so I'm not
>> sure, as a domain with no paging mode enabled could still have a paging
>> pool configured.
> 
> I can gate this on total_pages > 0 if that would address your concern
> with paging_mode_enabled().
> 
> However, regarding the gating itself, I don't believe that the line
> saved by not printing this
> is worth the potential confusion of somebody looking for this
> information and not finding
> it, I would prefer to leave it printing all zeroes.

While I can see your point, I also have to state that already on earlier
additions to the (bulky) output I was concerned of the overall (and
long term) output size growth. Especially when transmitted over a slow
serial line, any shortening counts imo. Anyway, I'm not going to insist,
first and foremost because I expect I'm the only one with this concern.

Somewhat related: With HVM=n and SHADOW_PAGING=n, this output isn't
useful at all, even if non-zero values were logged. All non-zero values
would indicate then is that someone pointlessly set up a paging pool
for a domain.

Jan

Re: [PATCH] x86/domain: Dump domain paging pool sizes with the rest of the pageframe info
Posted by Andrew Cooper 3 months, 1 week ago

On 24/07/2025 12:40 pm, Aidan Allen wrote:
> Added the ability to view the paging pool size via the debug keys
>
> Signed-off-by: Aidan Allen <aidan.allen1@cloud.com>

You should have CC'd all the x86 maintainers.  Adding Jan and Roger.

> ---
>  xen/arch/x86/domain.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 503a55705b..b64d09a9c8 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -253,6 +253,10 @@ void dump_pageframe_info(struct domain *d)
>                 page->count_info, page->u.inuse.type_info);
>      }
>
> +    printk("    Domain paging pool: total: %d, free: %d, p2m: %d\n",
> +           d->arch.paging.total_pages, d->arch.paging.free_pages,
> +           d->arch.paging.p2m_pages);

The %d's should be %u's, as each of these are unsigned quantities.  This
can be fixed on commit.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> +
>      nrspin_unlock(&d->page_alloc_lock);
>  }
>
> --
> 2.47.1