From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rob Gardner Subject: Re: [PATCH] Changes to allow dynamic enabling/disabling of trace buffers Date: Sun, 30 Oct 2005 15:59:26 -0700 Message-ID: <4365504E.8050808@hp.com> References: <436537C9.1090301@hp.com> <43654ED6.5070604@blueyonder.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <43654ED6.5070604@blueyonder.co.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: david.hopwood@blueyonder.co.uk Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org 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. > > >