WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: david.hopwood@xxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH] Changes to allow dynamic enabling/disabling of trace buffers
From: Rob Gardner <rob.gardner@xxxxxx>
Date: Sun, 30 Oct 2005 15:59:26 -0700
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Sun, 30 Oct 2005 22:58:45 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <43654ED6.5070604@xxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <436537C9.1090301@xxxxxx> <43654ED6.5070604@xxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla Thunderbird 1.0.2 (Windows/20050317)
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