Re: [Xen-devel] [PATCH v3 3/4] xen: refactor debugtrace data

[Juergen Gross <jgross@xxxxxxxx>]

On 03.09.19 13:50, Jan Beulich wrote:
> On 03.09.2019 12:31, Juergen Gross wrote:
> > On 03.09.19 12:16, Jan Beulich wrote:
> > > On 28.08.2019 10:00, Juergen Gross wrote:
> > > > +static unsigned int debugtrace_kilobytes = 128;
> > > Since you touch this anyway, add __initdata? Maybe also move it
> > > next to its integer_param()?
> > This is modified in patch 4 again, and there I need it in the running
> > system for support of cpu hotplug with per-cpu buffers.
> Right, I've meanwhile noticed this. Hence it's fine to keep as is.
>
> > > > @@ -165,12 +171,14 @@ static int __init debugtrace_init(void)
> > > >            return 0;
> > > >    
> > > >        order = get_order_from_bytes(bytes);
> > > > -    debugtrace_buf = alloc_xenheap_pages(order, 0);
> > > > -    ASSERT(debugtrace_buf != NULL);
> > > > +    data = alloc_xenheap_pages(order, 0);
> > > > +    if ( !data )
> > > > +        return -ENOMEM;
> > > >    
> > > > -    memset(debugtrace_buf, '\0', bytes);
> > > > +    memset(data, '\0', bytes);
> > > >    
> > > > -    debugtrace_bytes = bytes;
> > > > +    data->bytes = bytes - sizeof(*data);
> > > > +    debtr_data = data;
> > > The allocation may end up being almost twice as big as what gets
> > > actually used this way. Wouldn't it make sense to re-calculate
> > > "bytes" from "order"?
> > Yes, you are right.
> Actually I wasn't, which I did notice seeing the relevant piece
> of code getting touched in patch 4:
>
>      while ( (kbytes = (debugtrace_kilobytes & (debugtrace_kilobytes-1))) != 0 )
>          debugtrace_kilobytes = kbytes;
For kbytes < 4 you still were right.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel