Every guest_printk() call computes "(d%d) " prefix on every call.
Move prefix generation to the domain creation time.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/pv/shim.c     | 2 ++
 xen/common/domain.c        | 2 ++
 xen/drivers/char/console.c | 6 +-----
 xen/include/xen/sched.h    | 1 +
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 81e4a0516d..b9c034ccff 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -329,6 +329,8 @@ int pv_shim_shutdown(uint8_t reason)
 
     /* Update domain id. */
     d->domain_id = get_initial_domain_id();
+    snprintf(d->domain_prefix, sizeof(d->domain_prefix),
+             "(d%d) ", d->domain_id);
 
     /* Clean the iomem range. */
     BUG_ON(iomem_deny_access(d, 0, ~0UL));
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 0c4cc77111..49d4cb8221 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -681,6 +681,8 @@ struct domain *domain_create(domid_t domid,
 
     /* Sort out our idea of is_system_domain(). */
     d->domain_id = domid;
+    snprintf(d->domain_prefix, sizeof(d->domain_prefix),
+             "(d%d) ", d->domain_id);
     d->unique_id = get_unique_id();
 
     /* Holding CDF_* internal flags. */
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 235d55bbff..2e23910dfa 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -965,12 +965,8 @@ void printk(const char *fmt, ...)
 void guest_printk(const struct domain *d, const char *fmt, ...)
 {
     va_list args;
-    char prefix[16];
-
-    snprintf(prefix, sizeof(prefix), "(d%d) ", d->domain_id);
-
     va_start(args, fmt);
-    vprintk_common(prefix, fmt, args);
+    vprintk_common(d->domain_prefix, fmt, args);
     va_end(args);
 }
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 037c83fda2..e90921dbd1 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -368,6 +368,7 @@ struct evtchn_port_ops;
 struct domain
 {
     domid_t          domain_id;
+    char             domain_prefix[16]; /* '(dX) ' prefix for printouts */
 
     unsigned int     max_vcpus;
 
-- 
2.34.1

On 13/02/2025 10:35 pm, dmkhn@proton.me wrote:
> Every guest_printk() call computes "(d%d) " prefix on every call.
> Move prefix generation to the domain creation time.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

I'm on the fence here.

Part of that is speaking as someone who has had to shrink struct domain
several times to keep it fitting within 1 page.

But as to calculating it every time, does that matter?  In production
environments, we get a handful of print lines per domain across their
lifetime.  Is the saving really worth it?

~Andrew

On 14.02.2025 00:28, Andrew Cooper wrote:
> On 13/02/2025 10:35 pm, dmkhn@proton.me wrote:
>> Every guest_printk() call computes "(d%d) " prefix on every call.
>> Move prefix generation to the domain creation time.
>>
>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> 
> I'm on the fence here.
> 
> Part of that is speaking as someone who has had to shrink struct domain
> several times to keep it fitting within 1 page.

Just wanted to mention exactly this as well. I'm pretty strongly against
us doing anything like this. If we started here, likely other things
possible to "cache" would show up.

> But as to calculating it every time, does that matter?  In production
> environments, we get a handful of print lines per domain across their
> lifetime.  Is the saving really worth it?

+1

Jan

On Thursday, February 13th, 2025 at 3:28 PM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:

>
>
> On 13/02/2025 10:35 pm, dmkhn@proton.me wrote:
>
> > Every guest_printk() call computes "(d%d) " prefix on every call.
> > Move prefix generation to the domain creation time.
> >
> > Signed-off-by: Denis Mukhin dmukhin@ford.com
>
>
> I'm on the fence here.
>
> Part of that is speaking as someone who has had to shrink struct domain
> several times to keep it fitting within 1 page.
>
> But as to calculating it every time, does that matter? In production
> environments, we get a handful of print lines per domain across their
> lifetime. Is the saving really worth it?

Our setup should support domain restarts with heavy logging enabled.
While restarts are not expected to happen very often, when restart happens
the system shall boot to operational state pretty quickly.

Also, I was planning to use this code to address the feedback from:
  https://lore.kernel.org/xen-devel/cKowJ0lJhKcoHoaPgGOX4xdDu6PCcg7MVnhS_y5L4mVGJfNlG-xXJdSGXJkIys5OqdCeSdiYtNQmI4znkjXLaqtqHefgvM33MbvMX700nk0=@proton.me/

The code (unposted) is here:
  https://gitlab.com/xen-project/people/dmukhin/xen/-/commit/bf72477b77a09853c69319afed5280bff4eabb1d#f29178524efff3dcdb2342a5a4e5affb5fe99fd1

>
> ~Andrew