Re: [PATCH v5 5/6] xen/console: update conring memory allocation
On 05.02.2026 02:36, [email protected] wrote: > --- a/xen/drivers/char/console.c > +++ b/xen/drivers/char/console.c > @@ -463,20 +463,34 @@ static void cf_check conring_dump_keyhandler(unsigned > char key) > void __init console_init_ring(void) > { > char *ring; > -unsigned int start, size, chunk, order, memflags; > +unsigned int start, size, chunk; > unsigned long flags; > > if ( !opt_conring_size ) > return; > > -order = get_order_from_bytes(max(opt_conring_size, conring_size)); > -memflags = MEMF_bits(crashinfo_maxaddr_bits); > -while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL ) > +opt_conring_size = max(opt_conring_size, conring_size); > +size = ROUNDDOWN(opt_conring_size, PAGE_SIZE); > +if ( size != opt_conring_size ) > { > -BUG_ON(order == 0); > -order--; > +opt_conring_size = size; > +printk(XENLOG_WARNING "Rounding down console ring size to multiple > of %lu KiB.\n", > + PAGE_SIZE >> 10); > } > -opt_conring_size = PAGE_SIZE << order; I've spotted this removal only while looking at patch 6: How does this work? We require conring_size to be a power of 2, or else masking by (conring_size - 1) isn't a valid thing to do. You even touch CONRING_IDX_MASK() twice in this series, so you really should have noticed. > +if ( opt_conring_size >= GB(2) ) > +{ > +opt_conring_size = GB(2); > +printk(XENLOG_WARNING "Limiting user-configured console ring > size.\n"); > +} > +else if ( opt_conring_size < _CONRING_SIZE ) > +{ > +opt_conring_size = _CONRING_SIZE; > +printk(XENLOG_WARNING "Using compile-time console ring size.\n"); > +} > + > +/* Contiguous buffer; does not need to be naturally aligned. */ > +ring = xmalloc_bytes(opt_conring_size); > +BUG_ON(ring == NULL); > > nrspin_lock_irqsave(&console_lock, flags); >
Re: [PATCH v5 5/6] xen/console: update conring memory allocation
On 05.02.2026 02:36, [email protected] wrote: > --- a/xen/drivers/char/console.c > +++ b/xen/drivers/char/console.c > @@ -463,20 +463,34 @@ static void cf_check conring_dump_keyhandler(unsigned > char key) > void __init console_init_ring(void) > { > char *ring; > -unsigned int start, size, chunk, order, memflags; > +unsigned int start, size, chunk; > unsigned long flags; > > if ( !opt_conring_size ) > return; > > -order = get_order_from_bytes(max(opt_conring_size, conring_size)); > -memflags = MEMF_bits(crashinfo_maxaddr_bits); > -while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL ) > +opt_conring_size = max(opt_conring_size, conring_size); > +size = ROUNDDOWN(opt_conring_size, PAGE_SIZE); > +if ( size != opt_conring_size ) > { > -BUG_ON(order == 0); > -order--; > +opt_conring_size = size; > +printk(XENLOG_WARNING "Rounding down console ring size to multiple > of %lu KiB.\n", > + PAGE_SIZE >> 10); > } > -opt_conring_size = PAGE_SIZE << order; > +if ( opt_conring_size >= GB(2) ) > +{ > +opt_conring_size = GB(2); > +printk(XENLOG_WARNING "Limiting user-configured console ring > size.\n"); > +} > +else if ( opt_conring_size < _CONRING_SIZE ) > +{ > +opt_conring_size = _CONRING_SIZE; > +printk(XENLOG_WARNING "Using compile-time console ring size.\n"); > +} > + > +/* Contiguous buffer; does not need to be naturally aligned. */ > +ring = xmalloc_bytes(opt_conring_size); I'm sorry, but I'm going to veto any new uses of xmalloc_bytes(). As per the comment at the top of xvmalloc.h, the family of functions there should be used in new code. That family deliberately doesn't include a counterpart of xmalloc_bytes(). You're wanting a multiple of page size anyway, so perhaps it is warranted here to actually use vmalloc() directly. Jan
