[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6] xen/console: introduce domain_console struct
On 10.07.2025 03:35, dmkhn@xxxxxxxxx wrote: > @@ -877,6 +873,16 @@ struct domain *domain_create(domid_t domid, > > /* All error paths can depend on the above setup. */ > > + BUILD_BUG_ON(DOMAIN_CONSOLE_BUF_SIZE <= 0); While the "equals 0" case can in principle happen, the "less than" part is dead code (and hence this needs checking differently): The type of DOMAIN_CONSOLE_BUF_SIZE is an unsigned one, so wrapping through 0 will yield huge positive values. > + err = -ENOMEM; > + d->console = xzalloc_bytes(DOMAIN_CONSOLE_SIZE); As previously indicated, new code ought to use the xv*alloc family of functions, which deliberately doesn't include any ..._bytes() forms. Note how instead there is xvzalloc_flex_struct() for situations like the one here. > @@ -371,6 +373,26 @@ struct evtchn_port_ops; > > #define MAX_NR_IOREQ_SERVERS 8 > > +/* > + * Domain console settings is the dynamically-allocated data structure. > + * Using an even multiple of a cache line size may help to optimize the > + * allocation overhead. > + */ > +#define DOMAIN_CONSOLE_SIZE ROUNDUP(256, SMP_CACHE_BYTES) > +#define DOMAIN_CONSOLE_BUF_SIZE (DOMAIN_CONSOLE_SIZE - \ > + sizeof(struct domain_console)) But you're aware that there's allocation overhead, which consumes part of a cacheline? I simply don't understand why this struct is so different from others that such cleverness needs building in. Yet if it's relevant, it really needs doing correctly. > +/* Domain console settings. */ > +struct domain_console { > + /* Permission to take ownership of the physical console input. */ > + bool input_allowed; > + > + /* hvm_print_line() and guest_console_write() logging. */ > + unsigned int idx; > + spinlock_t lock; > + char buf[0]; Iirc recent gcc warns about the use of this historic gcc extension, since there has been the better form using just [] for quite a long time. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |