[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

-- 
David Hopwood <david.nospam.hopwood@xxxxxxxxxxxxxxxx>


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.