Re: [Xen-devel] [PATCH] Changes to allow dynamic enabling/disabling of trace buffers

[Rob Gardner <rob.gardner@xxxxxx>]

David,

Thanks for your comments. I'll make the changes you suggest.

Rob


David Hopwood wrote:

> Rob Gardner wrote:
>
> /**
> * tb_set_size - handle the logic involved with dynamically
> * allocating and deallocating tbufs
> *
> * This function is called when the SET_SIZE hypercall is done.
> */
> int tb_set_size(int size)
> {
>    // There are three cases to handle:
>    //  1. Changing from 0 to non-zero ==> simple allocate
>    //  2. Changing from non-zero to 0 ==> simple deallocate
>    //  3. Changing size ==> deallocate and reallocate? Or disallow?
>    //     User can just do a change to 0, then a change to the new size.
>
> They can, but the interface is slightly simpler if you allow this, and
> the implementation is also slightly shorter because you don't have to
> check for the error, or comment on it:
>
>    if (opt_tbuf_size > 0) {
>        int order = get_order_from_pages(num_online_cpus() * opt_tbuf_size);
>        // is there a way to undo SHARE_PFN_WITH_DOMAIN?
>        free_xenheap_pages(t_bufs[0], order);
>        opt_tbuf_size = 0;
>        printk("Xen trace buffers: uninitialized\n");
>    }
>    if (size > 0) {
>        // What if size is too big? alloc_xenheap will complain.
>        opt_tbuf_size = size;
>        if (alloc_trace_bufs() != 0)
>            return -EINVAL;
>        wmb();
>        printk("Xen trace buffers: initialized\n");
>    }
>    return 0;
> }
>
> The sentence "To change the size of an existing allocation, you must first
> deallocate it then reallocate it." should also be removed from the
> xc_tbuf_set_size doc comment.
>
> As a matter of style, I would also suggest moving the tb_init_done check into
> tb_set_size rather than doing it in the DOM0_TBUF_SET_SIZE handler.
>
>  

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel