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

[Rob Gardner <rob.gardner@hp.com>]

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.
>
>  
>