Re: [PATCH v2] MMIO: Make coalesced mmio use a device per zone

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

 



----- Original Message -----
> On 07/19/2011 02:05 PM, Sasha Levin wrote:
> > On Tue, 2011-07-19 at 13:57 +0300, Avi Kivity wrote:
> > >  On 07/19/2011 01:31 PM, Sasha Levin wrote:
> > >  >  This patch changes coalesced mmio to create one mmio device
> > >  >  per
> > >  >  zone instead of handling all zones in one device.
> > >  >
> > >  >  Doing so enables us to take advantage of existing locking and
> > >  >  prevents
> > >  >  a race condition between coalesced mmio
> > >  >  registration/unregistration
> > >  >  and lookups.
> > >  >
> > >  >  @@ -63,7 +63,7 @@ extern struct kmem_cache *kvm_vcpu_cache;
> > >  >     */
> > >  >    struct kvm_io_bus {
> > >  >    	int                   dev_count;
> > >  >  -#define NR_IOBUS_DEVS 200
> > >  >  +#define NR_IOBUS_DEVS 300
> > >  >    	struct kvm_io_device *devs[NR_IOBUS_DEVS];
> > >  >    };
> > >
> > >  This means that a lot of non-coalesced-mmio users can squeeze
> > >  out
> > >  coalesced-mmio.  I don't know if it's really worthwhile, but the
> > >  100
> > >  coalesced mmio slots should be reserved so we are guaranteed
> > >  they are
> > >  available.
> >
> > We are currently registering 4 devices, plus how many
> > ioeventfds/coalesced mmio zones the user wants. I felt bad about
> > upping
> > it to 300 really.
> 
> It's just a few kilobytes, where even a small guest occupies half a
> gigabyte.  Even just its pagetables swallow up megabytes.
> 
> An array means less opportunities to screw up the code and better
> cache
> usage with small objects.


In commit e80e2a60, it increase NR_IOBUS_DEVS because virtio-net(vhost=on)
device requests 2 iobus devices.

> commit e80e2a60ff7914dae691345a976c80bbbff3ec74
> Author: Sridhar Samudrala <sri@xxxxxxxxxx>
> Date:   Tue Mar 30 16:48:25 2010 -0700
> 
>     KVM: Increase NR_IOBUS_DEVS limit to 200
>     
>     This patch increases the current hardcoded limit of NR_IOBUS_DEVS
>     from 6 to 200. We are hitting this limit when creating a guest with more
>     than 1 virtio-net device using vhost-net backend. Each virtio-net
>     device requires 2 such devices to service notifications from rx/tx queues.
>     
>     Signed-off-by: Sridhar Samudrala <sri@xxxxxxxxxx>
>     Signed-off-by: Avi Kivity <avi@xxxxxxxxxx>

Hi all,

#define KVM_COALESCED_MMIO_ZONE_MAX 100
Old maximum of coalesced mmio zone is 100. After apply this patch,
each zone has an iobus devices, so the NR_IOBUS_DEVS is increased by 100.

> commit 8c99ce360904ba2f46d4840f1b8df7451331ea38
> Author: Sasha Levin <levinsasha928@xxxxxxxxx>
> Date:   Wed Jul 20 20:59:00 2011 +0300
> 
>     KVM: Make coalesced mmio use a device per zone
>     
>     This patch changes coalesced mmio to create one mmio device per
>     zone instead of handling all zones in one device.
>     
>     Doing so enables us to take advantage of existing locking and prevents
>     a race condition between coalesced mmio registration/unregistration
>     and lookups.
>     
>     Suggested-by: Avi Kivity <avi@xxxxxxxxxx>
>     Signed-off-by: Sasha Levin <levinsasha928@xxxxxxxxx>
>     Signed-off-by: Marcelo Tosatti <mtosatti@xxxxxxxxxx>


kvm_io_bus devices are used for ioevent, pit, pic, ioapic, coalesced_mmio. 

virt/kvm/coalesced_mmio.c:144:  ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, zone->addr,               -->  kvm_vm_ioctl_register_coalesced_mmio()
virt/kvm/eventfd.c:589: ret = kvm_io_bus_register_dev(kvm, bus_idx, p->addr, p->length,                    -->  kvm_assign_ioeventfd()
virt/kvm/ioapic.c:397:  ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, ioapic->base_address,             -->  kvm_ioapic_init()
arch/x86/kvm/i8254.c:716:       ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, KVM_PIT_BASE_ADDRESS,      -->  kvm_create_pit()
arch/x86/kvm/i8254.c:723:               ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS,
arch/x86/kvm/i8259.c:613:       ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x20, 2,                   ---> kvm_create_pic()
arch/x86/kvm/i8259.c:618:       ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0xa0, 2, &s->dev_slave);
arch/x86/kvm/i8259.c:622:       ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x4d0, 2, &s->dev_eclr);


Each virtio-blk device needs 1 iobus device ?
Each virtio-net(vhost=on) device needs 2 iobus devices?

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, '300' IOBUS devices are enough?

Thanks, Amos


> > >  >
> > >  >  @@ -95,6 +85,8 @@ static void
> > >  >  coalesced_mmio_destructor(struct kvm_io_device *this)
> > >  >    {
> > >  >    	struct kvm_coalesced_mmio_dev *dev = to_mmio(this);
> > >  >
> > >  >  +	list_del(&dev->list);
> > >  >  +
> > >  >    	kfree(dev);
> > >  >    }
> > >  >
> > >
> > >  No lock?
> >
> > The lock is there to synchronize access to the coalesced ring (it
> > was
> > here before this patch too, it's not something new), not the device
> > list.
> >
> > The device list is only accessed when kvm->slots_lock is held, so
> > it
> > takes care of that.
> 
> Right.  A comment please.
> 
> btw, don't we leak all zones on guest destruction? the array didn't
> need
> any cleanup, but this list does.
> 
> --
> 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
> 
--
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