| Message ID | 20250213214350.1745843-1-dmukhin@ford.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen/console: pre-compute domain prefix for printouts | expand |
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 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
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
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;
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(-)