Re: [PATCH v6 1/2] KVM: resize kvm_io_range array dynamically

[Amos Kong <akong@redhat.com>]

On 09/03/12 07:20, Marcelo Tosatti wrote:
> On Thu, Mar 08, 2012 at 10:03:55AM +0800, Amos Kong wrote:
>> This patch makes the kvm_io_range array can be resized dynamically.
>>
>> Signed-off-by: Amos Kong<akong@redhat.com>
>> ---
>>   include/linux/kvm_host.h |    5 +++--
>>   virt/kvm/kvm_main.c      |   38 ++++++++++++++++++--------------------
>>   2 files changed, 21 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index e42d85a..8a6c1a3 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -67,10 +67,11 @@ struct kvm_io_range {
>>   	struct kvm_io_device *dev;
>>   };
>>
>> +#define NR_IOBUS_DEVS 300
>> +
>>   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 94e148e..f6ee1e2 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -2393,9 +2393,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,
>> @@ -2495,12 +2492,15 @@ 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)
>>   		return -ENOMEM;
>> +	memcpy(new_bus, bus, sizeof(*bus) + (bus->dev_count *
>> +	       sizeof(struct kvm_io_range)));
>>   	kvm_io_bus_insert_dev(new_bus, dev, addr, len);
>>   	rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
>>   	synchronize_srcu_expedited(&kvm->srcu);
>> @@ -2517,27 +2517,25 @@ 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) {

Hi Marcelo,

the deleted dev is found here.

>>   			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 = kzalloc(sizeof(*bus) + ((bus->dev_count - 1) *
>> +			  sizeof(struct kvm_io_range)), GFP_KERNEL);
>> +	if (!new_bus)
>> +		return -ENOMEM;
>> +


>> +	new_bus->dev_count--;
>
> Was just zeroed above?
>
>> +	memcpy(new_bus->range, bus->range, i * sizeof(struct kvm_io_range));

Oh, dev_count memory needs to be copied.

         memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct 
kvm_io_range));
         new_bus->dev_count--;


>> +	memcpy(new_bus->range + i, bus->range + i + 1,
>> +	       (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
>
> Did you mean "i" as an indice for ->range[] ?

'i' is the index of deleted dev in bus->range[]


-- 
			Amos.