Re: [PATCH v3] kvm: Use a bitmap for tracking used GSIs

From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alex Williamson <alex.williamson@hp.com>
Cc: kvm@vger.kernel.org, sheng.yang@intel.com
Subject: Re: [PATCH v3] kvm: Use a bitmap for tracking used GSIs
Date: Wed, 13 May 2009 10:04:51 +0300	[thread overview]
Message-ID: <20090513070451.GB31139@redhat.com> (raw)
In-Reply-To: <20090512220142.5663.72948.stgit@dl380g6-3.ned.telco.ned.telco>

On Tue, May 12, 2009 at 04:07:15PM -0600, Alex Williamson wrote:
> We're currently using a counter to track the most recent GSI we've
> handed out.  This quickly hits KVM_MAX_IRQ_ROUTES when using device
> assignment with a driver that regularly toggles the MSI enable bit.
> This can mean only a few minutes of usable run time.  Instead, track
> used GSIs in a bitmap.
> 
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> ---
> 
>  v2: Added mutex to protect gsi bitmap
>  v3: Updated for comments from Michael Tsirkin
>      No longer depends on "[PATCH] kvm: device-assignment: Catch GSI overflow"
> 
>  hw/device-assignment.c  |    4 ++
>  kvm/libkvm/kvm-common.h |    4 ++
>  kvm/libkvm/libkvm.c     |   83 +++++++++++++++++++++++++++++++++++++++++------
>  kvm/libkvm/libkvm.h     |   10 ++++++
>  4 files changed, 88 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index a7365c8..a6cc9b9 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -561,8 +561,10 @@ static void free_dev_irq_entries(AssignedDevice *dev)
>  {
>      int i;
>  
> -    for (i = 0; i < dev->irq_entries_nr; i++)
> +    for (i = 0; i < dev->irq_entries_nr; i++) {
>          kvm_del_routing_entry(kvm_context, &dev->entry[i]);
> +        kvm_free_irq_route_gsi(kvm_context, dev->entry[i].gsi);
> +    }
>      free(dev->entry);
>      dev->entry = NULL;
>      dev->irq_entries_nr = 0;
> diff --git a/kvm/libkvm/kvm-common.h b/kvm/libkvm/kvm-common.h
> index 591fb53..4b3cb51 100644
> --- a/kvm/libkvm/kvm-common.h
> +++ b/kvm/libkvm/kvm-common.h
> @@ -66,8 +66,10 @@ struct kvm_context {
>  #ifdef KVM_CAP_IRQ_ROUTING
>  	struct kvm_irq_routing *irq_routes;
>  	int nr_allocated_irq_routes;
> +	void *used_gsi_bitmap;
> +	int max_gsi;
> +	pthread_mutex_t gsi_mutex;
>  #endif
> -	int max_used_gsi;
>  };
>  
>  int kvm_alloc_kernel_memory(kvm_context_t kvm, unsigned long memory,
> diff --git a/kvm/libkvm/libkvm.c b/kvm/libkvm/libkvm.c
> index ba0a5d1..3d7ab75 100644
> --- a/kvm/libkvm/libkvm.c
> +++ b/kvm/libkvm/libkvm.c
> @@ -35,6 +35,7 @@
>  #include <errno.h>
>  #include <sys/ioctl.h>
>  #include <inttypes.h>
> +#include <pthread.h>
>  #include "libkvm.h"
>  
>  #if defined(__x86_64__) || defined(__i386__)
> @@ -65,6 +66,8 @@
>  int kvm_abi = EXPECTED_KVM_API_VERSION;
>  int kvm_page_size;
>  
> +static inline void set_bit(uint32_t *buf, unsigned int bit);
> +
>  struct slot_info {
>  	unsigned long phys_addr;
>  	unsigned long len;
> @@ -286,6 +289,9 @@ kvm_context_t kvm_init(struct kvm_callbacks *callbacks,
>  	int fd;
>  	kvm_context_t kvm;
>  	int r;
> +#ifdef KVM_CAP_IRQ_ROUTING

Let's kill all these ifdefs. Or at least, let's not add them.

-- 
MST