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] xentrace: dynamic tracebuffer size allocation

To: Olaf Hering <olaf@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xentrace: dynamic tracebuffer size allocation
From: Keir Fraser <keir@xxxxxxx>
Date: Sat, 05 Feb 2011 20:35:14 +0000
Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
Delivery-date: Sat, 05 Feb 2011 12:36:15 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:sender:user-agent:date:subject:from:to:cc :message-id:thread-topic:thread-index:in-reply-to:mime-version :content-type:content-transfer-encoding; bh=Xkst7zYe8/J0X3zn6RckgcYGw/u3IzA2GpBPrBfq8ZA=; b=OwbUG+zkFSW6OyMtHDCMFIK6Bj4vE9lCz6yAZYlilXL+NOnvGKTW8cRNvqqrh74XMq ksnMXKQhjY3+/2LmeAEKWNxiifVGFupVQpZKlgCqUoV5rwhew10vhtmH9S6MKfzjuG8B ksyw3Lt9/phVEdvDa19J3nPNg5cr+sYzADiYk=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:user-agent:date:subject:from:to:cc:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; b=NAUCMm86aSCuEweeFUmO6paO0i+YqUTE1EdfMJHYSWytauXcnfDIABoLzZ1m3c21kS vhHrj2jjZAQF41++Dqq1Dv9FzUD0I11qWimtk/turkdF8a45R/rZG7jd2P1kHxL+R+F3 dJxyO9jJYDabppiio9fev/T9jaC+VMPtBge/g=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110205140717.GA3224@xxxxxxxxx>
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/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcvFdDEMBYBvtjIb60eo2gGdF4g4Jg==
Thread-topic: [Xen-devel] [PATCH] xentrace: dynamic tracebuffer size allocation
User-agent: Microsoft-Entourage/12.28.0.101117
On 05/02/2011 14:07, "Olaf Hering" <olaf@xxxxxxxxx> wrote:

> 
> Allocate tracebuffers dynamically, based on the requested buffer size.
> Calculate t_info_size from requested t_buf size.
> Fix allocation failure path, free pages without the spinlock.
> The spinlock is not needed since tracing is not yet enabled.
> Remove casts for rawbuf, it can be a void pointer since no math is done.

Bit big for 4.1 now I think. Needs an Ack from George Dunlap also.

 -- Keir

> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> 
> ---
>  xen/common/trace.c |  214
> ++++++++++++++++++++++-------------------------------
>  1 file changed, 91 insertions(+), 123 deletions(-)
> 
> --- xen-unstable.hg-4.1.22870.orig/xen/common/trace.c
> +++ xen-unstable.hg-4.1.22870/xen/common/trace.c
> @@ -42,14 +42,14 @@ CHECK_t_buf;
>  #define compat_t_rec t_rec
>  #endif
>  
> -/* opt_tbuf_size: trace buffer size (in pages) */
> -static unsigned int opt_tbuf_size = 0;
> +/* opt_tbuf_size: trace buffer size (in pages) for each cpu */
> +static unsigned int opt_tbuf_size;
>  integer_param("tbuf_size", opt_tbuf_size);
>  
>  /* Pointers to the meta-data objects for all system trace buffers */
>  static struct t_info *t_info;
> -#define T_INFO_PAGES 2  /* Size fixed at 2 pages for now. */
> -#define T_INFO_SIZE ((T_INFO_PAGES)*(PAGE_SIZE))
> +static unsigned int t_info_pages;
> +
>  static DEFINE_PER_CPU_READ_MOSTLY(struct t_buf *, t_bufs);
>  static DEFINE_PER_CPU_READ_MOSTLY(unsigned char *, t_data);
>  static DEFINE_PER_CPU_READ_MOSTLY(spinlock_t, t_lock);
> @@ -78,6 +78,21 @@ static u32 tb_event_mask = TRC_ALL;
>   * i.e., sizeof(_type) * ans >= _x. */
>  #define fit_to_type(_type, _x) (((_x)+sizeof(_type)-1) / sizeof(_type))
>  
> +static int cpu_callback(
> +    struct notifier_block *nfb, unsigned long action, void *hcpu)
> +{
> +    unsigned int cpu = (unsigned long)hcpu;
> +
> +    if ( action == CPU_UP_PREPARE )
> +        spin_lock_init(&per_cpu(t_lock, cpu));
> +
> +    return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block cpu_nfb = {
> +    .notifier_call = cpu_callback
> +};
> +
>  static void calc_tinfo_first_offset(void)
>  {
>      int offset_in_bytes = offsetof(struct t_info, mfn_offset[NR_CPUS]);
> @@ -85,20 +100,30 @@ static void calc_tinfo_first_offset(void
>  }
>  
>  /**
> - * check_tbuf_size - check to make sure that the proposed size will fit
> + * calculate_tbuf_size - check to make sure that the proposed size will fit
>   * in the currently sized struct t_info and allows prod and cons to
>   * reach double the value without overflow.
>   */
> -static int check_tbuf_size(u32 pages)
> +static int calculate_tbuf_size(unsigned int pages)
>  {
>      struct t_buf dummy;
> -    typeof(dummy.prod) size;
> -    
> -    size = ((typeof(dummy.prod))pages)  * PAGE_SIZE;
> -    
> -    return (size / PAGE_SIZE != pages)
> -           || (size + size < size)
> -           || (num_online_cpus() * pages + t_info_first_offset > T_INFO_SIZE
> / sizeof(uint32_t));
> +    typeof(dummy.prod) size = -1;
> +
> +    /* max size holds up to n pages */
> +    size /= PAGE_SIZE;
> +    if ( pages > size )
> +    {
> +        gdprintk(XENLOG_INFO, "%s: requested number of %u pages reduced to
> %u\n",
> +               __func__, pages, (unsigned int)size);
> +        pages = size;
> +    }
> +
> +    t_info_pages = num_online_cpus() * pages + t_info_first_offset;
> +    t_info_pages *= sizeof(uint32_t);
> +    t_info_pages /= PAGE_SIZE;
> +    if ( t_info_pages % PAGE_SIZE )
> +        t_info_pages++;
> +    return pages;
>  }
>  
>  /**
> @@ -111,47 +136,48 @@ static int check_tbuf_size(u32 pages)
>   * This function may also be called later when enabling trace buffers
>   * via the SET_SIZE hypercall.
>   */
> -static int alloc_trace_bufs(void)
> +static int alloc_trace_bufs(unsigned int pages)
>  {
> -    int           i, cpu, order;
> -    unsigned long nr_pages;
> +    int i, cpu, order;
>      /* Start after a fixed-size array of NR_CPUS */
>      uint32_t *t_info_mfn_list;
>      int offset;
>  
> -    if ( opt_tbuf_size == 0 )
> -        return -EINVAL;
> -
> -    if ( check_tbuf_size(opt_tbuf_size) )
> +    if ( t_info )
>      {
> -        printk("Xen trace buffers: tb size %d too large. "
> -               "Tracing disabled.\n",
> -               opt_tbuf_size);
> -        return -EINVAL;
> +        printk("Xen trace buffers: t_info already allocated.\n");
> +        return -EBUSY;
>      }
>  
> -    /* t_info size is fixed for now. Currently this works great, so there
> -     * seems to be no need to make it dynamic. */
> -    t_info = alloc_xenheap_pages(get_order_from_pages(T_INFO_PAGES), 0);
> +    if ( pages == 0 )
> +        return -EINVAL;
> +
> +    /* Calculate offset in u32 of first mfn */
> +    calc_tinfo_first_offset();
> +
> +    pages = calculate_tbuf_size(pages);
> +    order = get_order_from_pages(pages);
> +
> +    t_info = alloc_xenheap_pages(get_order_from_pages(t_info_pages), 0);
>      if ( t_info == NULL )
>      {
> -        printk("Xen trace buffers: t_info allocation failed! "
> -               "Tracing disabled.\n");
> +        printk("Xen trace buffers: t_info allocation failed! Tracing
> disabled.\n");
>          return -ENOMEM;
>      }
>  
> -    for ( i = 0; i < T_INFO_PAGES; i++ )
> -        share_xen_page_with_privileged_guests(
> -            virt_to_page(t_info) + i, XENSHARE_readonly);
> -
> -    t_info_mfn_list = (uint32_t *)t_info;
>      offset = t_info_first_offset;
> +    t_info_mfn_list = (uint32_t *)t_info;
>  
> -    t_info->tbuf_size = opt_tbuf_size;
> -    printk(XENLOG_INFO "tbuf_size %d\n", t_info->tbuf_size);
> +    /* Per-cpu t_lock initialisation. */
> +    for_each_online_cpu ( cpu )
> +        spin_lock_init(&per_cpu(t_lock, cpu));
> +    register_cpu_notifier(&cpu_nfb);
>  
> -    nr_pages = opt_tbuf_size;
> -    order = get_order_from_pages(nr_pages);
> +    for(i = 0; i < t_info_pages; i++)
> +        share_xen_page_with_privileged_guests(
> +            virt_to_page(t_info) + i, XENSHARE_readonly);
> +
> +    t_info->tbuf_size = pages;
>  
>      /*
>       * First, allocate buffers for all of the cpus.  If any
> @@ -160,25 +186,19 @@ static int alloc_trace_bufs(void)
>      for_each_online_cpu(cpu)
>      {
>          int flags;
> -        char         *rawbuf;
> +        void *rawbuf;
>          struct t_buf *buf;
>  
>          if ( (rawbuf = alloc_xenheap_pages(
>                  order, MEMF_bits(32 + PAGE_SHIFT))) == NULL )
>          {
> -            printk("Xen trace buffers: memory allocation failed\n");
> -            opt_tbuf_size = 0;
> +            printk("Xen trace buffers: memory allocation failed on cpu %d\n",
> cpu);
>              goto out_dealloc;
>          }
>  
> -        spin_lock_irqsave(&per_cpu(t_lock, cpu), flags);
> -
> -        per_cpu(t_bufs, cpu) = buf = (struct t_buf *)rawbuf;
> +        per_cpu(t_bufs, cpu) = buf = rawbuf;
>          buf->cons = buf->prod = 0;
>          per_cpu(t_data, cpu) = (unsigned char *)(buf + 1);
> -
> -        spin_unlock_irqrestore(&per_cpu(t_lock, cpu), flags);
> -
>      }
>  
>      /*
> @@ -188,14 +208,14 @@ static int alloc_trace_bufs(void)
>      for_each_online_cpu(cpu)
>      {
>          /* Share pages so that xentrace can map them. */
> -        char         *rawbuf;
> +        void *rawbuf;
>  
> -        if ( (rawbuf = (char *)per_cpu(t_bufs, cpu)) )
> +        if ( (rawbuf = per_cpu(t_bufs, cpu)) )
>          {
>              struct page_info *p = virt_to_page(rawbuf);
>              uint32_t mfn = virt_to_mfn(rawbuf);
>  
> -            for ( i = 0; i < nr_pages; i++ )
> +            for ( i = 0; i < pages; i++ )
>              {
>                  share_xen_page_with_privileged_guests(
>                      p + i, XENSHARE_writable);
> @@ -211,24 +231,28 @@ static int alloc_trace_bufs(void)
>          }
>      }
>  
> -    data_size  = (opt_tbuf_size * PAGE_SIZE - sizeof(struct t_buf));
> +    data_size  = (pages * PAGE_SIZE - sizeof(struct t_buf));
>      t_buf_highwater = data_size >> 1; /* 50% high water */
> +    opt_tbuf_size = pages;
> +
> +    printk("Xen trace buffers: initialised\n");
> +    wmb(); /* above must be visible before tb_init_done flag set */
> +    tb_init_done = 1;
>  
>      return 0;
>  out_dealloc:
>      for_each_online_cpu(cpu)
>      {
>          int flags;
> -        char * rawbuf;
> +        void *rawbuf;
>  
> -        spin_lock_irqsave(&per_cpu(t_lock, cpu), flags);
> -        if ( (rawbuf = (char *)per_cpu(t_bufs, cpu)) )
> +        rawbuf = per_cpu(t_bufs, cpu);
> +        per_cpu(t_bufs, cpu) = NULL;
> +        if ( rawbuf )
>          {
> -            per_cpu(t_bufs, cpu) = NULL;
>              ASSERT(!(virt_to_page(rawbuf)->count_info & PGC_allocated));
>              free_xenheap_pages(rawbuf, order);
>          }
> -        spin_unlock_irqrestore(&per_cpu(t_lock, cpu), flags);
>      }
>      
>      return -ENOMEM;
> @@ -236,41 +260,25 @@ out_dealloc:
>  
>  
>  /**
> - * tb_set_size - handle the logic involved with dynamically
> - * allocating and deallocating tbufs
> + * tb_set_size - handle the logic involved with dynamically allocating tbufs
>   *
>   * This function is called when the SET_SIZE hypercall is done.
>   */
> -static int tb_set_size(int size)
> +static int tb_set_size(unsigned int pages)
>  {
>      /*
>       * Setting size is a one-shot operation. It can be done either at
>       * boot time or via control tools, but not by both. Once buffers
>       * are created they cannot be destroyed.
>       */
> -    int ret = 0;
> -
> -    if ( opt_tbuf_size != 0 )
> +    if ( opt_tbuf_size && pages != opt_tbuf_size )
>      {
> -        if ( size != opt_tbuf_size )
> -            gdprintk(XENLOG_INFO, "tb_set_size from %d to %d not
> implemented\n",
> -                     opt_tbuf_size, size);
> +        gdprintk(XENLOG_INFO, "tb_set_size from %d to %d not implemented\n",
> +                     opt_tbuf_size, pages);
>          return -EINVAL;
>      }
>  
> -    if ( size <= 0 )
> -        return -EINVAL;
> -
> -    opt_tbuf_size = size;
> -
> -    if ( (ret = alloc_trace_bufs()) != 0 )
> -    {
> -        opt_tbuf_size = 0;
> -        return ret;
> -    }
> -
> -    printk("Xen trace buffers: initialized\n");
> -    return 0;
> +    return alloc_trace_bufs(pages);
>  }
>  
>  int trace_will_trace_event(u32 event)
> @@ -299,21 +307,6 @@ int trace_will_trace_event(u32 event)
>      return 1;
>  }
>  
> -static int cpu_callback(
> -    struct notifier_block *nfb, unsigned long action, void *hcpu)
> -{
> -    unsigned int cpu = (unsigned long)hcpu;
> -
> -    if ( action == CPU_UP_PREPARE )
> -        spin_lock_init(&per_cpu(t_lock, cpu));
> -
> -    return NOTIFY_DONE;
> -}
> -
> -static struct notifier_block cpu_nfb = {
> -    .notifier_call = cpu_callback
> -};
> -
>  /**
>   * init_trace_bufs - performs initialization of the per-cpu trace buffers.
>   *
> @@ -323,37 +316,12 @@ static struct notifier_block cpu_nfb = {
>   */
>  void __init init_trace_bufs(void)
>  {
> -    int i;
> -
> -    /* Calculate offset in u32 of first mfn */
> -    calc_tinfo_first_offset();
> -
> -    /* Per-cpu t_lock initialisation. */
> -    for_each_online_cpu ( i )
> -        spin_lock_init(&per_cpu(t_lock, i));
> -    register_cpu_notifier(&cpu_nfb);
> -
> -    if ( opt_tbuf_size == 0 )
> -    {
> -        printk("Xen trace buffers: disabled\n");
> -        goto fail;
> -    }
> -
> -    if ( alloc_trace_bufs() != 0 )
> +    if ( opt_tbuf_size &&  alloc_trace_bufs(opt_tbuf_size) )
>      {
> -        dprintk(XENLOG_INFO, "Xen trace buffers: "
> -                "allocation size %d failed, disabling\n",
> -                opt_tbuf_size);
> -        goto fail;
> +        gdprintk(XENLOG_INFO, "Xen trace buffers: "
> +                 "allocation size %d failed, disabling\n",
> +                 opt_tbuf_size);
>      }
> -
> -    printk("Xen trace buffers: initialised\n");
> -    wmb(); /* above must be visible before tb_init_done flag set */
> -    tb_init_done = 1;
> -    return;
> -
> - fail:
> -    opt_tbuf_size = 0;
>  }
>  
>  /**
> @@ -372,7 +340,7 @@ int tb_control(xen_sysctl_tbuf_op_t *tbc
>      case XEN_SYSCTL_TBUFOP_get_info:
>          tbc->evt_mask   = tb_event_mask;
>          tbc->buffer_mfn = t_info ? virt_to_mfn(t_info) : 0;
> -        tbc->size = T_INFO_PAGES * PAGE_SIZE;
> +        tbc->size = t_info_pages * PAGE_SIZE;
>          break;
>      case XEN_SYSCTL_TBUFOP_set_cpu_mask:
>          rc = xenctl_cpumap_to_cpumask(&tb_cpu_mask, &tbc->cpu_mask);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



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