Re: [PATCH v4] KVM: Resize kvm_io_range array dynamically

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



----- Original Message -----
> On 03/01/2012 09:01 AM, Amos Kong wrote:
> > kvm_io_bus devices are used for ioevent, pit, pic, ioapic,
> > coalesced_mmio.
> >
> > Currently Qemu only emulates one PCI bus, it contains 32 slots,
> > one slot contains 8 functions, maximum of supported PCI devices:
> >  1 * 32 * 8 = 256. One virtio-blk takes one iobus device,
> > one virtio-net(vhost=on) takes two iobus devices.
> > The maximum of coalesced mmio zone is 100, each zone
> > has an iobus devices. So 300 io_bus devices are not enough.
> >
> > This patch makes the kvm_io_range array can be resized dynamically.
> > Set an upper bounds for kvm_io_range to limit userspace.
> > 1000 is a very large limit and not bloat the typical user.
> >
> 
> Please separate the change to 1000 devs to a new patch.

ok

> > +#define NR_IOBUS_DEVS 1000
> > +
> >  struct kvm_io_bus {
> >  	int                   dev_count;
> > -#define NR_IOBUS_DEVS 300
> > -	struct kvm_io_range range[NR_IOBUS_DEVS];
> > +	struct kvm_io_range range[];
> >  };
> >  
> >  enum kvm_bus {
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index e4431ad..1baed68 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2389,9 +2389,6 @@ int kvm_io_bus_sort_cmp(const void *p1, const
> > void *p2)
> >  int kvm_io_bus_insert_dev(struct kvm_io_bus *bus, struct
> >  kvm_io_device *dev,
> >  			  gpa_t addr, int len)
> >  {
> > -	if (bus->dev_count == NR_IOBUS_DEVS)
> > -		return -ENOSPC;
> > -
> >  	bus->range[bus->dev_count++] = (struct kvm_io_range) {
> >  		.addr = addr,
> >  		.len = len,
> > @@ -2491,10 +2488,14 @@ int kvm_io_bus_register_dev(struct kvm
> > *kvm, enum kvm_bus bus_idx, gpa_t addr,
> >  	struct kvm_io_bus *new_bus, *bus;
> >  
> >  	bus = kvm->buses[bus_idx];
> > -	if (bus->dev_count > NR_IOBUS_DEVS-1)
> > +	if (bus->dev_count > NR_IOBUS_DEVS - 1)
> >  		return -ENOSPC;
> >  
> > -	new_bus = kmemdup(bus, sizeof(struct kvm_io_bus), GFP_KERNEL);
> > +	new_bus = kzalloc(sizeof(*bus) + ((bus->dev_count + 1) *
> > +			  sizeof(struct kvm_io_range)), GFP_KERNEL);
> > +	if (new_bus)
> > +		memcpy(new_bus, bus, sizeof(*bus) + (bus->dev_count *
> > +		       sizeof(struct kvm_io_range)));
> 
> This will be cleaner if you move the memcmp() after the check just
> below.

nod.
 
> >  	if (!new_bus)
> >  		return -ENOMEM;
> >  	kvm_io_bus_insert_dev(new_bus, dev, addr, len);
> > @@ -2513,26 +2514,28 @@ int kvm_io_bus_unregister_dev(struct kvm
> > *kvm, enum kvm_bus bus_idx,
> >  	struct kvm_io_bus *new_bus, *bus;
> >  
> >  	bus = kvm->buses[bus_idx];
> > -
> > -	new_bus = kmemdup(bus, sizeof(*bus), GFP_KERNEL);
> > -	if (!new_bus)
> > -		return -ENOMEM;
> > -
> >  	r = -ENOENT;
> > -	for (i = 0; i < new_bus->dev_count; i++)
> > -		if (new_bus->range[i].dev == dev) {
> > +	for (i = 0; i < bus->dev_count; i++)
> > +		if (bus->range[i].dev == dev) {
> >  			r = 0;
> > -			new_bus->dev_count--;
> > -			new_bus->range[i] = new_bus->range[new_bus->dev_count];
> > -			sort(new_bus->range, new_bus->dev_count,
> > -			     sizeof(struct kvm_io_range),
> > -			     kvm_io_bus_sort_cmp, NULL);
> >  			break;
> >  		}
> >  
> > -	if (r) {
> > -		kfree(new_bus);
> > +	if (r)
> >  		return r;
> > +
> > +	new_bus = kmemdup(bus, sizeof(*bus) + ((bus->dev_count - 1) *
> > +			  sizeof(struct kvm_io_range)), GFP_KERNEL);
> > +	if (!new_bus)
> > +		return -ENOMEM;
> > +
> > +	new_bus->dev_count--;
> > +	/* copy last entry of bus->range to deleted entry spot if
> > +	   deleted entry isn't the last entry of bus->range */
> > +	if (i != bus->dev_count - 1) {
> 
> The check is unneeded - if they compare equal, the copy is a no-op.


In kvm_io_bus_unregister_dev(), we need to delete one entry from original bus array.
so the allocated new bus array only has $N - 1 entries, ($N is the entry number of original bus array)

If i equals to bus->dev_count - 1, then the entry which is need to be deleted is the last entry of original bus array.
and the entry isn't copied to new bus array, so we don't need to do anything, sort isn't necessary.

Amos.
 
> > +		new_bus->range[i] = bus->range[bus->dev_count - 1];
> 
> > +		sort(new_bus->range, new_bus->dev_count,
> > +		     sizeof(struct kvm_io_range), kvm_io_bus_sort_cmp, NULL);
> >  	}
> >  
> >  	rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
> >
> 
> --
> error compiling committee.c: too many arguments to function
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux