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.