From mboxrd@z Thu Jan 1 00:00:00 1970 From: Amos Kong Subject: Re: [PATCH v6 1/2] KVM: resize kvm_io_range array dynamically Date: Fri, 09 Mar 2012 12:05:08 +0800 Message-ID: <4F598174.9090405@redhat.com> References: <20120308020215.9413.63371.stgit@dhcp-8-167.nay.redhat.com> <20120308020355.9413.89411.stgit@dhcp-8-167.nay.redhat.com> <20120308232022.GA24937@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: jasowang@redhat.com, alex.williamson@redhat.com, kvm@vger.kernel.org, levinsasha928@gmail.com To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:44164 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754295Ab2CIDyc (ORCPT ); Thu, 8 Mar 2012 22:54:32 -0500 In-Reply-To: <20120308232022.GA24937@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: 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 >> --- >> 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.