From 568da4f8c43d2e5b614964c6aefd768de3e3af14 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 16 Jun 2014 11:50:44 +0200 Subject: [PATCH 1/9] pt-irq fixes and improvements Tools side: - don't silently ignore unrecognized PT_IRQ_TYPE_* values - respect that the interface type contains a union, making the code at once no longer depend on the hypervisor ignoring the bus field of the PCI portion of the interface structure) Hypervisor side: - don't ignore the PCI bus number passed in - don't store values (gsi, link) calculated from other stored values - avoid calling xfree() with a spin lock held where easily possible - have pt_irq_destroy_bind() respect the passed in type - scope reduction and constification of various variables - use switch instead of if/else-if chains - formatting Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper Acked-by: Yang Zhang Acked-by: Ian Campbell --- tools/libxc/xc_domain.c | 38 +++-- xen/arch/x86/physdev.c | 7 +- xen/drivers/passthrough/io.c | 193 ++++++++++++++------------ xen/drivers/passthrough/vtd/x86/vtd.c | 8 +- xen/include/asm-x86/hvm/io.h | 2 +- xen/include/xen/hvm/irq.h | 4 +- 6 files changed, 146 insertions(+), 106 deletions(-) diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index 37ed14114e..9c8653e79a 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -1702,16 +1702,21 @@ int xc_domain_bind_pt_irq( bind->hvm_domid = domid; bind->irq_type = irq_type; bind->machine_irq = machine_irq; - if ( irq_type == PT_IRQ_TYPE_PCI || - irq_type == PT_IRQ_TYPE_MSI_TRANSLATE ) + switch ( irq_type ) { + case PT_IRQ_TYPE_PCI: + case PT_IRQ_TYPE_MSI_TRANSLATE: bind->u.pci.bus = bus; - bind->u.pci.device = device; + bind->u.pci.device = device; bind->u.pci.intx = intx; - } - else if ( irq_type == PT_IRQ_TYPE_ISA ) + case PT_IRQ_TYPE_ISA: bind->u.isa.isa_irq = isa_irq; - + break; + default: + errno = EINVAL; + return -1; + } + rc = do_domctl(xch, &domctl); return rc; } @@ -1737,11 +1742,22 @@ int xc_domain_unbind_pt_irq( bind->hvm_domid = domid; bind->irq_type = irq_type; bind->machine_irq = machine_irq; - bind->u.pci.bus = bus; - bind->u.pci.device = device; - bind->u.pci.intx = intx; - bind->u.isa.isa_irq = isa_irq; - + switch ( irq_type ) + { + case PT_IRQ_TYPE_PCI: + case PT_IRQ_TYPE_MSI_TRANSLATE: + bind->u.pci.bus = bus; + bind->u.pci.device = device; + bind->u.pci.intx = intx; + break; + case PT_IRQ_TYPE_ISA: + bind->u.isa.isa_irq = isa_irq; + break; + default: + errno = EINVAL; + return -1; + } + rc = do_domctl(xch, &domctl); return rc; } diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c index f1783155c3..6b3201b078 100644 --- a/xen/arch/x86/physdev.c +++ b/xen/arch/x86/physdev.c @@ -37,9 +37,8 @@ static int physdev_hvm_map_pirq( switch ( type ) { case MAP_PIRQ_TYPE_GSI: { - struct hvm_irq_dpci *hvm_irq_dpci; - struct hvm_girq_dpci_mapping *girq; - uint32_t machine_gsi = 0; + const struct hvm_irq_dpci *hvm_irq_dpci; + unsigned int machine_gsi = 0; if ( *index < 0 || *index >= NR_HVM_IRQS ) { @@ -52,6 +51,8 @@ static int physdev_hvm_map_pirq( hvm_irq_dpci = domain_get_irq_dpci(d); if ( hvm_irq_dpci ) { + const struct hvm_girq_dpci_mapping *girq; + BUILD_BUG_ON(ARRAY_SIZE(hvm_irq_dpci->girq) < NR_HVM_IRQS); list_for_each_entry ( girq, &hvm_irq_dpci->girq[*index], diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c index f64e4ac775..a380c9cdca 100644 --- a/xen/drivers/passthrough/io.c +++ b/xen/drivers/passthrough/io.c @@ -51,11 +51,8 @@ static int pt_irq_guest_eoi(struct domain *d, struct hvm_pirq_dpci *pirq_dpci, static void pt_irq_time_out(void *data) { struct hvm_pirq_dpci *irq_map = data; - unsigned int guest_gsi; - struct hvm_irq_dpci *dpci = NULL; - struct dev_intx_gsi_link *digl; - struct hvm_girq_dpci_mapping *girq; - uint32_t device, intx; + const struct hvm_irq_dpci *dpci; + const struct dev_intx_gsi_link *digl; spin_lock(&irq_map->dom->event_lock); @@ -63,16 +60,16 @@ static void pt_irq_time_out(void *data) ASSERT(dpci); list_for_each_entry ( digl, &irq_map->digl_list, list ) { - guest_gsi = digl->gsi; + unsigned int guest_gsi = hvm_pci_intx_gsi(digl->device, digl->intx); + const struct hvm_girq_dpci_mapping *girq; + list_for_each_entry ( girq, &dpci->girq[guest_gsi], list ) { struct pirq *pirq = pirq_info(irq_map->dom, girq->machine_gsi); pirq_dpci(pirq)->flags |= HVM_IRQ_DPCI_EOI_LATCH; } - device = digl->device; - intx = digl->intx; - hvm_pci_intx_deassert(irq_map->dom, device, intx); + hvm_pci_intx_deassert(irq_map->dom, digl->device, digl->intx); } pt_pirq_iterate(irq_map->dom, pt_irq_guest_eoi, NULL); @@ -96,13 +93,9 @@ void free_hvm_irq_dpci(struct hvm_irq_dpci *dpci) int pt_irq_create_bind( struct domain *d, xen_domctl_bind_pt_irq_t *pt_irq_bind) { - struct hvm_irq_dpci *hvm_irq_dpci = NULL; + struct hvm_irq_dpci *hvm_irq_dpci; struct hvm_pirq_dpci *pirq_dpci; struct pirq *info; - uint32_t guest_gsi; - uint32_t device, intx, link; - struct dev_intx_gsi_link *digl; - struct hvm_girq_dpci_mapping *girq; int rc, pirq = pt_irq_bind->machine_irq; if ( pirq < 0 || pirq >= d->nr_pirqs ) @@ -113,6 +106,8 @@ int pt_irq_create_bind( hvm_irq_dpci = domain_get_irq_dpci(d); if ( hvm_irq_dpci == NULL ) { + unsigned int i; + hvm_irq_dpci = xzalloc(struct hvm_irq_dpci); if ( hvm_irq_dpci == NULL ) { @@ -122,7 +117,7 @@ int pt_irq_create_bind( softirq_tasklet_init( &hvm_irq_dpci->dirq_tasklet, hvm_dirq_assist, (unsigned long)d); - for ( int i = 0; i < NR_HVM_IRQS; i++ ) + for ( i = 0; i < NR_HVM_IRQS; i++ ) INIT_LIST_HEAD(&hvm_irq_dpci->girq[i]); d->arch.hvm_domain.irq.dpci = hvm_irq_dpci; @@ -136,7 +131,9 @@ int pt_irq_create_bind( } pirq_dpci = pirq_dpci(info); - if ( pt_irq_bind->irq_type == PT_IRQ_TYPE_MSI ) + switch ( pt_irq_bind->irq_type ) + { + case PT_IRQ_TYPE_MSI: { uint8_t dest, dest_mode; int dest_vcpu_id; @@ -169,15 +166,16 @@ int pt_irq_create_bind( { uint32_t mask = HVM_IRQ_DPCI_MACH_MSI | HVM_IRQ_DPCI_GUEST_MSI; - if ( (pirq_dpci->flags & mask) != mask) + if ( (pirq_dpci->flags & mask) != mask ) { - spin_unlock(&d->event_lock); - return -EBUSY; + spin_unlock(&d->event_lock); + return -EBUSY; } - /* if pirq is already mapped as vmsi, update the guest data/addr */ + /* If pirq is already mapped as vmsi, update guest data/addr. */ if ( pirq_dpci->gmsi.gvec != pt_irq_bind->u.msi.gvec || - pirq_dpci->gmsi.gflags != pt_irq_bind->u.msi.gflags) { + pirq_dpci->gmsi.gflags != pt_irq_bind->u.msi.gflags ) + { /* Directly clear pending EOIs before enabling new MSI info. */ pirq_guest_eoi(info); @@ -185,7 +183,7 @@ int pt_irq_create_bind( pirq_dpci->gmsi.gflags = pt_irq_bind->u.msi.gflags; } } - /* Caculate dest_vcpu_id for MSI-type pirq migration */ + /* Calculate dest_vcpu_id for MSI-type pirq migration. */ dest = pirq_dpci->gmsi.gflags & VMSI_DEST_ID_MASK; dest_mode = !!(pirq_dpci->gmsi.gflags & VMSI_DM_MASK); dest_vcpu_id = hvm_girq_dest_2_vcpu_id(d, dest, dest_mode); @@ -193,36 +191,37 @@ int pt_irq_create_bind( spin_unlock(&d->event_lock); if ( dest_vcpu_id >= 0 ) hvm_migrate_pirqs(d->vcpu[dest_vcpu_id]); + break; } - else + + case PT_IRQ_TYPE_PCI: + case PT_IRQ_TYPE_MSI_TRANSLATE: { - device = pt_irq_bind->u.pci.device; - intx = pt_irq_bind->u.pci.intx; - guest_gsi = hvm_pci_intx_gsi(device, intx); - link = hvm_pci_intx_link(device, intx); + unsigned int bus = pt_irq_bind->u.pci.bus; + unsigned int device = pt_irq_bind->u.pci.device; + unsigned int intx = pt_irq_bind->u.pci.intx; + unsigned int guest_gsi = hvm_pci_intx_gsi(device, intx); + unsigned int link = hvm_pci_intx_link(device, intx); + struct dev_intx_gsi_link *digl = xmalloc(struct dev_intx_gsi_link); + struct hvm_girq_dpci_mapping *girq = + xmalloc(struct hvm_girq_dpci_mapping); + + if ( !digl || !girq ) + { + spin_unlock(&d->event_lock); + xfree(girq); + xfree(digl); + return -ENOMEM; + } + hvm_irq_dpci->link_cnt[link]++; - digl = xmalloc(struct dev_intx_gsi_link); - if ( !digl ) - { - spin_unlock(&d->event_lock); - return -ENOMEM; - } - - girq = xmalloc(struct hvm_girq_dpci_mapping); - if ( !girq ) - { - xfree(digl); - spin_unlock(&d->event_lock); - return -ENOMEM; - } - + digl->bus = bus; digl->device = device; digl->intx = intx; - digl->gsi = guest_gsi; - digl->link = link; list_add_tail(&digl->list, &pirq_dpci->digl_list); + girq->bus = bus; girq->device = device; girq->intx = intx; girq->machine_gsi = pirq; @@ -261,12 +260,12 @@ int pt_irq_create_bind( kill_timer(&pirq_dpci->timer); pirq_dpci->dom = NULL; list_del(&girq->list); - xfree(girq); list_del(&digl->list); hvm_irq_dpci->link_cnt[link]--; pirq_dpci->flags = 0; pirq_cleanup_check(info, d); spin_unlock(&d->event_lock); + xfree(girq); xfree(digl); return rc; } @@ -276,33 +275,51 @@ int pt_irq_create_bind( if ( iommu_verbose ) dprintk(XENLOG_G_INFO, - "d%d: bind: m_gsi=%u g_gsi=%u device=%u intx=%u\n", - d->domain_id, pirq, guest_gsi, device, intx); + "d%d: bind: m_gsi=%u g_gsi=%u dev=%02x.%02x.%u intx=%u\n", + d->domain_id, pirq, guest_gsi, bus, + PCI_SLOT(device), PCI_FUNC(device), intx); + break; } + + default: + spin_unlock(&d->event_lock); + return -EOPNOTSUPP; + } + return 0; } int pt_irq_destroy_bind( struct domain *d, xen_domctl_bind_pt_irq_t *pt_irq_bind) { - struct hvm_irq_dpci *hvm_irq_dpci = NULL; + struct hvm_irq_dpci *hvm_irq_dpci; struct hvm_pirq_dpci *pirq_dpci; - uint32_t machine_gsi, guest_gsi; - uint32_t device, intx, link; + unsigned int machine_gsi = pt_irq_bind->machine_irq; + unsigned int bus = pt_irq_bind->u.pci.bus; + unsigned int device = pt_irq_bind->u.pci.device; + unsigned int intx = pt_irq_bind->u.pci.intx; + unsigned int guest_gsi = hvm_pci_intx_gsi(device, intx); + unsigned int link = hvm_pci_intx_link(device, intx); struct dev_intx_gsi_link *digl, *tmp; struct hvm_girq_dpci_mapping *girq; struct pirq *pirq; - machine_gsi = pt_irq_bind->machine_irq; - device = pt_irq_bind->u.pci.device; - intx = pt_irq_bind->u.pci.intx; - guest_gsi = hvm_pci_intx_gsi(device, intx); - link = hvm_pci_intx_link(device, intx); + switch ( pt_irq_bind->irq_type ) + { + case PT_IRQ_TYPE_PCI: + case PT_IRQ_TYPE_MSI_TRANSLATE: + break; + case PT_IRQ_TYPE_MSI: + return 0; + default: + return -EOPNOTSUPP; + } if ( iommu_verbose ) dprintk(XENLOG_G_INFO, - "d%d: unbind: m_gsi=%u g_gsi=%u device=%u intx=%u\n", - d->domain_id, machine_gsi, guest_gsi, device, intx); + "d%d: unbind: m_gsi=%u g_gsi=%u dev=%02x:%02x.%u intx=%u\n", + d->domain_id, machine_gsi, guest_gsi, bus, + PCI_SLOT(device), PCI_FUNC(device), intx); spin_lock(&d->event_lock); @@ -314,18 +331,28 @@ int pt_irq_destroy_bind( return -EINVAL; } - hvm_irq_dpci->link_cnt[link]--; - list_for_each_entry ( girq, &hvm_irq_dpci->girq[guest_gsi], list ) { - if ( girq->machine_gsi == machine_gsi ) + if ( girq->bus == bus && + girq->device == device && + girq->intx == intx && + girq->machine_gsi == machine_gsi ) { - list_del(&girq->list); - xfree(girq); - break; + list_del(&girq->list); + xfree(girq); + girq = NULL; + break; } } + if ( girq ) + { + spin_unlock(&d->event_lock); + return -EINVAL; + } + + hvm_irq_dpci->link_cnt[link]--; + pirq = pirq_info(d, machine_gsi); pirq_dpci = pirq_dpci(pirq); @@ -334,10 +361,9 @@ int pt_irq_destroy_bind( { list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list ) { - if ( digl->device == device && - digl->intx == intx && - digl->link == link && - digl->gsi == guest_gsi ) + if ( digl->bus == bus && + digl->device == device && + digl->intx == intx ) { list_del(&digl->list); xfree(digl); @@ -359,8 +385,9 @@ int pt_irq_destroy_bind( if ( iommu_verbose ) dprintk(XENLOG_G_INFO, - "d%d unmap: m_irq=%u device=%u intx=%u\n", - d->domain_id, machine_gsi, device, intx); + "d%d unmap: m_irq=%u dev=%02x:%02x.%u intx=%u\n", + d->domain_id, machine_gsi, bus, + PCI_SLOT(device), PCI_FUNC(device), intx); return 0; } @@ -481,11 +508,10 @@ static void hvm_pci_msi_assert( static int _hvm_dirq_assist(struct domain *d, struct hvm_pirq_dpci *pirq_dpci, void *arg) { - uint32_t device, intx; - struct dev_intx_gsi_link *digl; - if ( test_and_clear_bool(pirq_dpci->masked) ) { + const struct dev_intx_gsi_link *digl; + if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI ) { hvm_pci_msi_assert(d, pirq_dpci); @@ -496,12 +522,10 @@ static int _hvm_dirq_assist(struct domain *d, struct hvm_pirq_dpci *pirq_dpci, { struct pirq *info = dpci_pirq(pirq_dpci); - device = digl->device; - intx = digl->intx; if ( hvm_domain_use_pirq(d, info) ) send_guest_pirq(d, info); else - hvm_pci_intx_assert(d, device, intx); + hvm_pci_intx_assert(d, digl->device, digl->intx); pirq_dpci->pending++; if ( pirq_dpci->flags & HVM_IRQ_DPCI_TRANSLATE ) @@ -537,16 +561,13 @@ static void hvm_dirq_assist(unsigned long _d) } static void __hvm_dpci_eoi(struct domain *d, - struct hvm_girq_dpci_mapping *girq, - union vioapic_redir_entry *ent) + const struct hvm_girq_dpci_mapping *girq, + const union vioapic_redir_entry *ent) { - uint32_t device, intx; struct pirq *pirq; struct hvm_pirq_dpci *pirq_dpci; - device = girq->device; - intx = girq->intx; - hvm_pci_intx_deassert(d, device, intx); + hvm_pci_intx_deassert(d, girq->device, girq->intx); pirq = pirq_info(d, girq->machine_gsi); pirq_dpci = pirq_dpci(pirq); @@ -556,8 +577,8 @@ static void __hvm_dpci_eoi(struct domain *d, * since interrupt is still not EOIed */ if ( --pirq_dpci->pending || - ( ent && ent->fields.mask ) || - ! pt_irq_need_timer(pirq_dpci->flags) ) + (ent && ent->fields.mask) || + !pt_irq_need_timer(pirq_dpci->flags) ) return; stop_timer(&pirq_dpci->timer); @@ -565,10 +586,10 @@ static void __hvm_dpci_eoi(struct domain *d, } void hvm_dpci_eoi(struct domain *d, unsigned int guest_gsi, - union vioapic_redir_entry *ent) + const union vioapic_redir_entry *ent) { - struct hvm_irq_dpci *hvm_irq_dpci; - struct hvm_girq_dpci_mapping *girq; + const struct hvm_irq_dpci *hvm_irq_dpci; + const struct hvm_girq_dpci_mapping *girq; if ( !iommu_enabled ) return; diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c index 9a458eb5e3..109234eaae 100644 --- a/xen/drivers/passthrough/vtd/x86/vtd.c +++ b/xen/drivers/passthrough/vtd/x86/vtd.c @@ -69,11 +69,13 @@ static int _hvm_dpci_isairq_eoi(struct domain *d, { struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq; unsigned int isairq = (long)arg; - struct dev_intx_gsi_link *digl, *tmp; + const struct dev_intx_gsi_link *digl; - list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list ) + list_for_each_entry ( digl, &pirq_dpci->digl_list, list ) { - if ( hvm_irq->pci_link.route[digl->link] == isairq ) + unsigned int link = hvm_pci_intx_link(digl->device, digl->intx); + + if ( hvm_irq->pci_link.route[link] == isairq ) { hvm_pci_intx_deassert(d, digl->device, digl->intx); if ( --pirq_dpci->pending == 0 ) diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h index 86db58de0a..8228ab8cd7 100644 --- a/xen/include/asm-x86/hvm/io.h +++ b/xen/include/asm-x86/hvm/io.h @@ -123,7 +123,7 @@ int handle_pio(uint16_t port, unsigned int size, int dir); void hvm_interrupt_post(struct vcpu *v, int vector, int type); void hvm_io_assist(ioreq_t *p); void hvm_dpci_eoi(struct domain *d, unsigned int guest_irq, - union vioapic_redir_entry *ent); + const union vioapic_redir_entry *ent); void msix_write_completion(struct vcpu *); struct hvm_hw_stdvga { diff --git a/xen/include/xen/hvm/irq.h b/xen/include/xen/hvm/irq.h index a8710dfdc5..c89f4b158f 100644 --- a/xen/include/xen/hvm/irq.h +++ b/xen/include/xen/hvm/irq.h @@ -30,10 +30,9 @@ struct dev_intx_gsi_link { struct list_head list; + uint8_t bus; uint8_t device; uint8_t intx; - uint8_t gsi; - uint8_t link; }; #define _HVM_IRQ_DPCI_MACH_PCI_SHIFT 0 @@ -69,6 +68,7 @@ struct hvm_gmsi_info { struct hvm_girq_dpci_mapping { struct list_head list; + uint8_t bus; uint8_t device; uint8_t intx; uint8_t machine_gsi; From ebbb51dc8c1790e5187442a808003298b6796762 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 16 Jun 2014 11:52:34 +0200 Subject: [PATCH 2/9] x86/EFI: improve boot time diagnostics (try 2) To aid analysis of eventual errors, print EFI status codes with error messages where available. Also remove a case where the status gets stored into a local variable without being used examined (which mis- guided me to add an error check there in try 1 of this patch). Signed-off-by: Jan Beulich --- xen/arch/x86/efi/boot.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/xen/arch/x86/efi/boot.c b/xen/arch/x86/efi/boot.c index a772509343..2b515f2953 100644 --- a/xen/arch/x86/efi/boot.c +++ b/xen/arch/x86/efi/boot.c @@ -344,11 +344,12 @@ static EFI_FILE_HANDLE __init get_parent_handle(EFI_LOADED_IMAGE *loaded_image, ret = efi_bs->HandleProtocol(loaded_image->DeviceHandle, &fs_protocol, (void **)&fio); if ( EFI_ERROR(ret) ) - blexit(L"Couldn't obtain the File System Protocol Interface"); + PrintErrMesg(L"Couldn't obtain the File System Protocol Interface", + ret); ret = fio->OpenVolume(fio, &dir_handle); } while ( ret == EFI_MEDIA_CHANGED ); if ( ret != EFI_SUCCESS ) - blexit(L"OpenVolume failure"); + PrintErrMesg(L"OpenVolume failure", ret); #define buffer ((CHAR16 *)keyhandler_scratch) #define BUFFERSIZE sizeof(keyhandler_scratch) @@ -967,8 +968,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL, (void **)&shim_lock)) && - shim_lock->Verify(kernel.ptr, kernel.size) != EFI_SUCCESS ) - blexit(L"Dom0 kernel image could not be verified."); + (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS ) + PrintErrMesg(L"Dom0 kernel image could not be verified", status); name.s = get_value(&cfg, section.s, "ramdisk"); if ( name.s ) @@ -1379,8 +1380,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) } } - status = efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, - &efi_mdesc_size, &mdesc_ver); + efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, + &efi_mdesc_size, &mdesc_ver); mbi.mem_upper -= efi_memmap_size; mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); if ( mbi.mem_upper < xen_phys_start ) @@ -1389,7 +1390,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, &efi_mdesc_size, &mdesc_ver); if ( EFI_ERROR(status) ) - blexit(L"Cannot obtain memory map"); + PrintErrMesg(L"Cannot obtain memory map", status); /* Populate E820 table and check trampoline area availability. */ e = e820map - 1; From 775c681db114672088e506b52c851db7456913f2 Mon Sep 17 00:00:00 2001 From: Ross Lagerwall Date: Mon, 16 Jun 2014 11:59:05 +0200 Subject: [PATCH 3/9] cpuidle: improve perf for certain workloads The existing mechanism of using interrupt frequency as a heuristic does not work well for certain workloads. As an example, synchronous dd on a small block size uses deep C-states because much of the time is spent doing processing so the interrupt frequency is not too high, but when an IOP is submitted, the interrupt occurs soon after going idle. This causes exit latency to be a significant factor. To fix this, add a new factor which limits the exit latency to be no more than 10% of the decaying measured idle time. This improves performance for workloads with a medium interrupt frequency but a short idle duration. In the workload given previously, throughput improves by 20% with this patch. This is not ported from the Linux menu governor since that uses load average and number of IO wait processes to satisfy latency constraints. If a process is in IO wait state, it compares the exit latency with the predicted residency reduced by a factor of 10, which is somewhat similar to what this patch does. A side effect of this patch is to correctly limit the maximum idle time used in the correction factor calculation. Previously data->measured_us was used, and it was never set. Signed-off-by: Ross Lagerwall --- xen/arch/x86/acpi/cpuidle_menu.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/xen/arch/x86/acpi/cpuidle_menu.c b/xen/arch/x86/acpi/cpuidle_menu.c index 69527766d6..4afaa8da25 100644 --- a/xen/arch/x86/acpi/cpuidle_menu.c +++ b/xen/arch/x86/acpi/cpuidle_menu.c @@ -36,6 +36,7 @@ #define RESOLUTION 1024 #define DECAY 4 #define MAX_INTERESTING 50000 +#define LATENCY_MULTIPLIER 10 /* * Concepts and ideas behind the menu governor @@ -88,6 +89,9 @@ * the average interrupt interval is, the smaller C state latency should be * and thus the less likely a busy CPU will hit such a deep C state. * + * As an additional rule to reduce the performance impact, menu tries to + * limit the exit latency duration to be no more than 10% of the decaying + * measured idle time. */ struct perf_factor{ @@ -102,6 +106,7 @@ struct menu_device int last_state_idx; unsigned int expected_us; u64 predicted_us; + u64 latency_factor; unsigned int measured_us; unsigned int exit_us; unsigned int bucket; @@ -199,6 +204,10 @@ static int menu_select(struct acpi_processor_power *power) io_interval = avg_intr_interval_us(); + data->latency_factor = DIV_ROUND( + data->latency_factor * (DECAY - 1) + data->measured_us, + DECAY); + /* * if the correction factor is 0 (eg first time init or cpu hotplug * etc), we actually want to start out with a unity factor. @@ -220,6 +229,8 @@ static int menu_select(struct acpi_processor_power *power) break; if (s->latency * IO_MULTIPLIER > io_interval) break; + if (s->latency * LATENCY_MULTIPLIER > data->latency_factor) + break; /* TBD: we need to check the QoS requirment in future */ data->exit_us = s->latency; data->last_state_idx = i; @@ -231,18 +242,16 @@ static int menu_select(struct acpi_processor_power *power) static void menu_reflect(struct acpi_processor_power *power) { struct menu_device *data = &__get_cpu_var(menu_devices); - unsigned int last_idle_us = power->last_residency; - unsigned int measured_us; u64 new_factor; - measured_us = last_idle_us; + data->measured_us = power->last_residency; /* * We correct for the exit latency; we are assuming here that the * exit latency happens after the event that we're interested in. */ - if (measured_us > data->exit_us) - measured_us -= data->exit_us; + if (data->measured_us > data->exit_us) + data->measured_us -= data->exit_us; /* update our correction ratio */ @@ -250,7 +259,7 @@ static void menu_reflect(struct acpi_processor_power *power) * (DECAY - 1) / DECAY; if (data->expected_us > 0 && data->measured_us < MAX_INTERESTING) - new_factor += RESOLUTION * measured_us / data->expected_us; + new_factor += RESOLUTION * data->measured_us / data->expected_us; else /* * we were idle so long that we count it as a perfect From 323338f86fb6cd6f6dba4f59a84eed71b3552d21 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Mon, 16 Jun 2014 11:59:32 +0200 Subject: [PATCH 4/9] x86/mce: don't spam the console with "CPUx: Temperature z" If the machine has been quite busy it ends up with these messages printed on the hypervisor console: (XEN) CPU3: Temperature/speed normal (XEN) CPU1: Temperature/speed normal (XEN) CPU0: Temperature/speed normal (XEN) CPU1: Temperature/speed normal (XEN) CPU0: Temperature/speed normal (XEN) CPU2: Temperature/speed normal (XEN) CPU3: Temperature/speed normal (XEN) CPU0: Temperature/speed normal (XEN) CPU2: Temperature/speed normal (XEN) CPU3: Temperature/speed normal (XEN) CPU1: Temperature/speed normal (XEN) CPU0: Temperature above threshold (XEN) CPU0: Running in modulated clock mode (XEN) CPU1: Temperature/speed normal (XEN) CPU2: Temperature/speed normal (XEN) CPU3: Temperature/speed normal While the state changes are important, the non-altered state information is not needed. As such add a latch mechanism to only print the information if it has changed since the last update (and the hardware doesn't properly suppress redundant notifications). This was observed on Intel DQ67SW, BIOS SWQ6710H.86A.0066.2012.1105.1504 11/05/2012 Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Andrew Cooper Acked-by: Christoph Egger --- xen/arch/x86/cpu/mcheck/mce_intel.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c b/xen/arch/x86/cpu/mcheck/mce_intel.c index ad06efca03..bb4ce47ef9 100644 --- a/xen/arch/x86/cpu/mcheck/mce_intel.c +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c @@ -49,11 +49,15 @@ static int __read_mostly nr_intel_ext_msrs; #define INTEL_SRAR_INSTR_FETCH 0x150 #ifdef CONFIG_X86_MCE_THERMAL +#define MCE_RING 0x1 +static DEFINE_PER_CPU(int, last_state); + static void intel_thermal_interrupt(struct cpu_user_regs *regs) { uint64_t msr_content; unsigned int cpu = smp_processor_id(); static DEFINE_PER_CPU(s_time_t, next); + int *this_last_state; ack_APIC_irq(); @@ -62,13 +66,17 @@ static void intel_thermal_interrupt(struct cpu_user_regs *regs) per_cpu(next, cpu) = NOW() + MILLISECS(5000); rdmsrl(MSR_IA32_THERM_STATUS, msr_content); - if (msr_content & 0x1) { - printk(KERN_EMERG "CPU%d: Temperature above threshold\n", cpu); - printk(KERN_EMERG "CPU%d: Running in modulated clock mode\n", - cpu); + this_last_state = &per_cpu(last_state, cpu); + if ( *this_last_state == (msr_content & MCE_RING) ) + return; + *this_last_state = msr_content & MCE_RING; + if ( msr_content & MCE_RING ) + { + printk(KERN_EMERG "CPU%u: Temperature above threshold\n", cpu); + printk(KERN_EMERG "CPU%u: Running in modulated clock mode\n", cpu); add_taint(TAINT_MACHINE_CHECK); } else { - printk(KERN_INFO "CPU%d: Temperature/speed normal\n", cpu); + printk(KERN_INFO "CPU%u: Temperature/speed normal\n", cpu); } } @@ -802,6 +810,7 @@ static int cpu_mcabank_alloc(unsigned int cpu) per_cpu(no_cmci_banks, cpu) = cmci; per_cpu(mce_banks_owned, cpu) = owned; + per_cpu(last_state, cpu) = -1; return 0; out: From 7430a86de0c9bd126b441570e459f6e06413cbf6 Mon Sep 17 00:00:00 2001 From: Malcolm Crossley Date: Mon, 16 Jun 2014 12:02:00 +0200 Subject: [PATCH 5/9] spread boot time page scrubbing across all available CPU's The page scrubbing is done in 128MB chunks in lockstep across all the non-SMT CPU's. This allows for the boot CPU to hold the heap_lock whilst each chunk is being scrubbed and then release the heap_lock when the CPU's are finished scrubing their individual chunk. This allows for the heap_lock to not be held continously and for pending softirqs are to be serviced periodically across the CPU's. The page scrub memory chunks are allocated to the CPU's in a NUMA aware fashion to reduce socket interconnect overhead and improve performance. Specifically in the first phase we scrub at the same time on all the NUMA nodes that have CPUs - we also weed out the SMT threads so that we only use cores (that gives a 50% boost). The second phase is for NUMA nodes that have no CPUs - for that we use the closest NUMA node's CPUs (non-SMT again) to do the job. This patch reduces the boot page scrub time on a 128GB 64 core AMD Opteron 6386 machine from 49 seconds to 3 seconds. On a IvyBridge-EX 8 socket box with 1.5TB it cuts it down from 15 minutes to 63 seconds. Signed-off-by: Malcolm Crossley Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Tim Deegan Reviewed-by: Andrew Cooper --- docs/misc/xen-command-line.markdown | 10 ++ xen/common/page_alloc.c | 218 +++++++++++++++++++++++++--- xen/include/asm-arm/numa.h | 1 + 3 files changed, 209 insertions(+), 20 deletions(-) diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 25829fe8ed..94f04bde33 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -198,6 +198,16 @@ Scrub free RAM during boot. This is a safety feature to prevent accidentally leaking sensitive VM data into other VMs if Xen crashes and reboots. +### `bootscrub_chunk` +> `= ` + +> Default: `128M` + +Maximum RAM block size chunks to be scrubbed whilst holding the page heap lock +and not running softirqs. Reduce this if softirqs are not being run frequently +enough. Setting this to a high value may cause boot failure, particularly if +the NMI watchdog is also enabled. + ### cachesize > `= ` diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 601319cc92..1b2d541ef8 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -64,6 +64,13 @@ string_param("badpage", opt_badpage); static bool_t opt_bootscrub __initdata = 1; boolean_param("bootscrub", opt_bootscrub); +/* + * bootscrub_chunk -> Amount of bytes to scrub lockstep on non-SMT CPUs + * on all NUMA nodes. + */ +static unsigned long __initdata opt_bootscrub_chunk = MB(128); +size_param("bootscrub_chunk", opt_bootscrub_chunk); + /* * Bit width of the DMA heap -- used to override NUMA-node-first. * allocation strategy, which can otherwise exhaust low memory. @@ -90,6 +97,16 @@ static struct bootmem_region { } *__initdata bootmem_region_list; static unsigned int __initdata nr_bootmem_regions; +struct scrub_region { + unsigned long offset; + unsigned long start; + unsigned long per_cpu_sz; + unsigned long rem; + cpumask_t cpus; +}; +static struct scrub_region __initdata region[MAX_NUMNODES]; +static unsigned long __initdata chunk_size; + static void __init boot_bug(int line) { panic("Boot BUG at %s:%d", __FILE__, line); @@ -1256,42 +1273,203 @@ void __init end_boot_allocator(void) printk("\n"); } +static void __init smp_scrub_heap_pages(void *data) +{ + unsigned long mfn, start, end; + struct page_info *pg; + struct scrub_region *r; + unsigned int temp_cpu, node, cpu_idx = 0; + unsigned int cpu = smp_processor_id(); + + if ( data ) + r = data; + else + { + node = cpu_to_node(cpu); + if ( node == NUMA_NO_NODE ) + return; + r = ®ion[node]; + } + + /* Determine the current CPU's index into CPU's linked to this node. */ + for_each_cpu ( temp_cpu, &r->cpus ) + { + if ( cpu == temp_cpu ) + break; + cpu_idx++; + } + + /* Calculate the starting mfn for this CPU's memory block. */ + start = r->start + (r->per_cpu_sz * cpu_idx) + r->offset; + + /* Calculate the end mfn into this CPU's memory block for this iteration. */ + if ( r->offset + chunk_size >= r->per_cpu_sz ) + { + end = r->start + (r->per_cpu_sz * cpu_idx) + r->per_cpu_sz; + + if ( r->rem && (cpumask_weight(&r->cpus) - 1 == cpu_idx) ) + end += r->rem; + } + else + end = start + chunk_size; + + for ( mfn = start; mfn < end; mfn++ ) + { + pg = mfn_to_page(mfn); + + /* Check the mfn is valid and page is free. */ + if ( !mfn_valid(mfn) || !page_state_is(pg, free) ) + continue; + + scrub_one_page(pg); + } +} + +static int __init find_non_smt(unsigned int node, cpumask_t *dest) +{ + cpumask_t node_cpus; + unsigned int i, cpu; + + cpumask_and(&node_cpus, &node_to_cpumask(node), &cpu_online_map); + cpumask_clear(dest); + for_each_cpu ( i, &node_cpus ) + { + if ( cpumask_intersects(dest, per_cpu(cpu_sibling_mask, i)) ) + continue; + cpu = cpumask_first(per_cpu(cpu_sibling_mask, i)); + cpumask_set_cpu(cpu, dest); + } + return cpumask_weight(dest); +} + /* - * Scrub all unallocated pages in all heap zones. This function is more - * convoluted than appears necessary because we do not want to continuously - * hold the lock while scrubbing very large memory areas. + * Scrub all unallocated pages in all heap zones. This function uses all + * online cpu's to scrub the memory in parallel. */ void __init scrub_heap_pages(void) { - unsigned long mfn; - struct page_info *pg; + cpumask_t node_cpus, all_worker_cpus; + unsigned int i, j; + unsigned long offset, max_per_cpu_sz = 0; + unsigned long start, end; + unsigned long rem = 0; + int last_distance, best_node; + int cpus; if ( !opt_bootscrub ) return; - printk("Scrubbing Free RAM: "); + cpumask_clear(&all_worker_cpus); + /* Scrub block size. */ + chunk_size = opt_bootscrub_chunk >> PAGE_SHIFT; + if ( chunk_size == 0 ) + chunk_size = MB(128) >> PAGE_SHIFT; - for ( mfn = first_valid_mfn; mfn < max_page; mfn++ ) + /* Round #0 - figure out amounts and which CPUs to use. */ + for_each_online_node ( i ) { + if ( !node_spanned_pages(i) ) + continue; + /* Calculate Node memory start and end address. */ + start = max(node_start_pfn(i), first_valid_mfn); + end = min(node_start_pfn(i) + node_spanned_pages(i), max_page); + /* Just in case NODE has 1 page and starts below first_valid_mfn. */ + end = max(end, start); + /* CPUs that are online and on this node (if none, that it is OK). */ + cpus = find_non_smt(i, &node_cpus); + cpumask_or(&all_worker_cpus, &all_worker_cpus, &node_cpus); + if ( cpus <= 0 ) + { + /* No CPUs on this node. Round #2 will take of it. */ + rem = 0; + region[i].per_cpu_sz = (end - start); + } + else + { + rem = (end - start) % cpus; + region[i].per_cpu_sz = (end - start) / cpus; + if ( region[i].per_cpu_sz > max_per_cpu_sz ) + max_per_cpu_sz = region[i].per_cpu_sz; + } + region[i].start = start; + region[i].rem = rem; + cpumask_copy(®ion[i].cpus, &node_cpus); + } + + printk("Scrubbing Free RAM on %d nodes using %d CPUs\n", num_online_nodes(), + cpumask_weight(&all_worker_cpus)); + + /* Round: #1 - do NUMA nodes with CPUs. */ + for ( offset = 0; offset < max_per_cpu_sz; offset += chunk_size ) + { + for_each_online_node ( i ) + region[i].offset = offset; + process_pending_softirqs(); - pg = mfn_to_page(mfn); + spin_lock(&heap_lock); + on_selected_cpus(&all_worker_cpus, smp_scrub_heap_pages, NULL, 1); + spin_unlock(&heap_lock); - /* Quick lock-free check. */ - if ( !mfn_valid(mfn) || !page_state_is(pg, free) ) + printk("."); + } + + /* + * Round #2: NUMA nodes with no CPUs get scrubbed with CPUs on the node + * closest to us and with CPUs. + */ + for_each_online_node ( i ) + { + node_cpus = node_to_cpumask(i); + + if ( !cpumask_empty(&node_cpus) ) continue; - /* Every 100MB, print a progress dot. */ - if ( (mfn % ((100*1024*1024)/PAGE_SIZE)) == 0 ) + last_distance = INT_MAX; + best_node = first_node(node_online_map); + /* Figure out which NODE CPUs are close. */ + for_each_online_node ( j ) + { + int distance; + + if ( cpumask_empty(&node_to_cpumask(j)) ) + continue; + + distance = __node_distance(i, j); + if ( distance < last_distance ) + { + last_distance = distance; + best_node = j; + } + } + /* + * Use CPUs from best node, and if there are no CPUs on the + * first node (the default) use the BSP. + */ + cpus = find_non_smt(best_node, &node_cpus); + if ( cpus == 0 ) + { + cpumask_set_cpu(smp_processor_id(), &node_cpus); + cpus = 1; + } + /* We already have the node information from round #0. */ + region[i].rem = region[i].per_cpu_sz % cpus; + region[i].per_cpu_sz /= cpus; + max_per_cpu_sz = region[i].per_cpu_sz; + cpumask_copy(®ion[i].cpus, &node_cpus); + + for ( offset = 0; offset < max_per_cpu_sz; offset += chunk_size ) + { + region[i].offset = offset; + + process_pending_softirqs(); + + spin_lock(&heap_lock); + on_selected_cpus(&node_cpus, smp_scrub_heap_pages, ®ion[i], 1); + spin_unlock(&heap_lock); + printk("."); - - spin_lock(&heap_lock); - - /* Re-check page status with lock held. */ - if ( page_state_is(pg, free) ) - scrub_one_page(pg); - - spin_unlock(&heap_lock); + } } printk("done.\n"); diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h index cb8f2ba39b..2c019d7f0b 100644 --- a/xen/include/asm-arm/numa.h +++ b/xen/include/asm-arm/numa.h @@ -12,6 +12,7 @@ static inline __attribute__((pure)) int phys_to_nid(paddr_t addr) /* XXX: implement NUMA support */ #define node_spanned_pages(nid) (total_pages) +#define node_start_pfn(nid) (frametable_base_mfn) #define __node_distance(a, b) (20) #endif /* __ARCH_ARM_NUMA_H */ From 3fe14e35d96991c095a7e6916ad07f0c31bc0fad Mon Sep 17 00:00:00 2001 From: Dario Faggioli Date: Mon, 16 Jun 2014 12:11:52 +0200 Subject: [PATCH 6/9] sched: rename v->cpu_affinity into v->cpu_hard_affinity in order to distinguish it from the cpu_soft_affinity which will be introduced a later commit ("xen: sched: introduce soft-affinity and use it instead d->node-affinity"). This patch does not imply any functional change, it is basically the result of something like the following: s/cpu_affinity/cpu_hard_affinity/g s/cpu_affinity_tmp/cpu_hard_affinity_tmp/g s/cpu_affinity_saved/cpu_hard_affinity_saved/g Signed-off-by: Dario Faggioli Reviewed-by: George Dunlap Acked-by: Jan Beulich --- xen/arch/x86/traps.c | 11 ++++++----- xen/common/domain.c | 22 +++++++++++----------- xen/common/domctl.c | 2 +- xen/common/keyhandler.c | 2 +- xen/common/sched_credit.c | 12 ++++++------ xen/common/sched_sedf.c | 2 +- xen/common/schedule.c | 21 +++++++++++---------- xen/common/wait.c | 4 ++-- xen/include/xen/sched.h | 8 ++++---- 9 files changed, 43 insertions(+), 41 deletions(-) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 44ac014e7f..cc1c87cff0 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -3173,7 +3173,8 @@ static void nmi_mce_softirq(void) /* Set the tmp value unconditionally, so that * the check in the iret hypercall works. */ - cpumask_copy(st->vcpu->cpu_affinity_tmp, st->vcpu->cpu_affinity); + cpumask_copy(st->vcpu->cpu_hard_affinity_tmp, + st->vcpu->cpu_hard_affinity); if ((cpu != st->processor) || (st->processor != st->vcpu->processor)) @@ -3208,11 +3209,11 @@ void async_exception_cleanup(struct vcpu *curr) return; /* Restore affinity. */ - if ( !cpumask_empty(curr->cpu_affinity_tmp) && - !cpumask_equal(curr->cpu_affinity_tmp, curr->cpu_affinity) ) + if ( !cpumask_empty(curr->cpu_hard_affinity_tmp) && + !cpumask_equal(curr->cpu_hard_affinity_tmp, curr->cpu_hard_affinity) ) { - vcpu_set_affinity(curr, curr->cpu_affinity_tmp); - cpumask_clear(curr->cpu_affinity_tmp); + vcpu_set_affinity(curr, curr->cpu_hard_affinity_tmp); + cpumask_clear(curr->cpu_hard_affinity_tmp); } if ( !(curr->async_exception_mask & (curr->async_exception_mask - 1)) ) diff --git a/xen/common/domain.c b/xen/common/domain.c index bc57174ba1..141a5dc2e4 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -125,9 +125,9 @@ struct vcpu *alloc_vcpu( tasklet_init(&v->continue_hypercall_tasklet, NULL, 0); - if ( !zalloc_cpumask_var(&v->cpu_affinity) || - !zalloc_cpumask_var(&v->cpu_affinity_tmp) || - !zalloc_cpumask_var(&v->cpu_affinity_saved) || + if ( !zalloc_cpumask_var(&v->cpu_hard_affinity) || + !zalloc_cpumask_var(&v->cpu_hard_affinity_tmp) || + !zalloc_cpumask_var(&v->cpu_hard_affinity_saved) || !zalloc_cpumask_var(&v->vcpu_dirty_cpumask) ) goto fail_free; @@ -156,9 +156,9 @@ struct vcpu *alloc_vcpu( fail_wq: destroy_waitqueue_vcpu(v); fail_free: - free_cpumask_var(v->cpu_affinity); - free_cpumask_var(v->cpu_affinity_tmp); - free_cpumask_var(v->cpu_affinity_saved); + free_cpumask_var(v->cpu_hard_affinity); + free_cpumask_var(v->cpu_hard_affinity_tmp); + free_cpumask_var(v->cpu_hard_affinity_saved); free_cpumask_var(v->vcpu_dirty_cpumask); free_vcpu_struct(v); return NULL; @@ -427,7 +427,7 @@ void domain_update_node_affinity(struct domain *d) for_each_vcpu ( d, v ) { - cpumask_and(online_affinity, v->cpu_affinity, online); + cpumask_and(online_affinity, v->cpu_hard_affinity, online); cpumask_or(cpumask, cpumask, online_affinity); } @@ -792,9 +792,9 @@ static void complete_domain_destroy(struct rcu_head *head) for ( i = d->max_vcpus - 1; i >= 0; i-- ) if ( (v = d->vcpu[i]) != NULL ) { - free_cpumask_var(v->cpu_affinity); - free_cpumask_var(v->cpu_affinity_tmp); - free_cpumask_var(v->cpu_affinity_saved); + free_cpumask_var(v->cpu_hard_affinity); + free_cpumask_var(v->cpu_hard_affinity_tmp); + free_cpumask_var(v->cpu_hard_affinity_saved); free_cpumask_var(v->vcpu_dirty_cpumask); free_vcpu_struct(v); } @@ -934,7 +934,7 @@ int vcpu_reset(struct vcpu *v) v->async_exception_mask = 0; memset(v->async_exception_state, 0, sizeof(v->async_exception_state)); #endif - cpumask_clear(v->cpu_affinity_tmp); + cpumask_clear(v->cpu_hard_affinity_tmp); clear_bit(_VPF_blocked, &v->pause_flags); clear_bit(_VPF_in_reset, &v->pause_flags); diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 4774277da1..b5c5c6ce3c 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -625,7 +625,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) else { ret = cpumask_to_xenctl_bitmap( - &op->u.vcpuaffinity.cpumap, v->cpu_affinity); + &op->u.vcpuaffinity.cpumap, v->cpu_hard_affinity); } } break; diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c index 5afcfefb45..d6eb0263dc 100644 --- a/xen/common/keyhandler.c +++ b/xen/common/keyhandler.c @@ -295,7 +295,7 @@ static void dump_domains(unsigned char key) !vcpu_event_delivery_is_enabled(v)); cpuset_print(tmpstr, sizeof(tmpstr), v->vcpu_dirty_cpumask); printk("dirty_cpus=%s ", tmpstr); - cpuset_print(tmpstr, sizeof(tmpstr), v->cpu_affinity); + cpuset_print(tmpstr, sizeof(tmpstr), v->cpu_hard_affinity); printk("cpu_affinity=%s\n", tmpstr); printk(" pause_count=%d pause_flags=%lx\n", atomic_read(&v->pause_count), v->pause_flags); diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index db5512e3ed..c6a2560abe 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -332,13 +332,13 @@ csched_balance_cpumask(const struct vcpu *vc, int step, cpumask_t *mask) if ( step == CSCHED_BALANCE_NODE_AFFINITY ) { cpumask_and(mask, CSCHED_DOM(vc->domain)->node_affinity_cpumask, - vc->cpu_affinity); + vc->cpu_hard_affinity); if ( unlikely(cpumask_empty(mask)) ) - cpumask_copy(mask, vc->cpu_affinity); + cpumask_copy(mask, vc->cpu_hard_affinity); } else /* step == CSCHED_BALANCE_CPU_AFFINITY */ - cpumask_copy(mask, vc->cpu_affinity); + cpumask_copy(mask, vc->cpu_hard_affinity); } static void burn_credits(struct csched_vcpu *svc, s_time_t now) @@ -407,7 +407,7 @@ __runq_tickle(unsigned int cpu, struct csched_vcpu *new) if ( balance_step == CSCHED_BALANCE_NODE_AFFINITY && !__vcpu_has_node_affinity(new->vcpu, - new->vcpu->cpu_affinity) ) + new->vcpu->cpu_hard_affinity) ) continue; /* Are there idlers suitable for new (for this balance step)? */ @@ -642,7 +642,7 @@ _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc, bool_t commit) /* Store in cpus the mask of online cpus on which the domain can run */ online = cpupool_scheduler_cpumask(vc->domain->cpupool); - cpumask_and(&cpus, vc->cpu_affinity, online); + cpumask_and(&cpus, vc->cpu_hard_affinity, online); for_each_csched_balance_step( balance_step ) { @@ -1498,7 +1498,7 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step) * or counter. */ if ( balance_step == CSCHED_BALANCE_NODE_AFFINITY - && !__vcpu_has_node_affinity(vc, vc->cpu_affinity) ) + && !__vcpu_has_node_affinity(vc, vc->cpu_hard_affinity) ) continue; csched_balance_cpumask(vc, balance_step, csched_balance_mask); diff --git a/xen/common/sched_sedf.c b/xen/common/sched_sedf.c index 0c9011a54e..7c80badedf 100644 --- a/xen/common/sched_sedf.c +++ b/xen/common/sched_sedf.c @@ -384,7 +384,7 @@ static int sedf_pick_cpu(const struct scheduler *ops, struct vcpu *v) cpumask_t *online; online = cpupool_scheduler_cpumask(v->domain->cpupool); - cpumask_and(&online_affinity, v->cpu_affinity, online); + cpumask_and(&online_affinity, v->cpu_hard_affinity, online); return cpumask_cycle(v->vcpu_id % cpumask_weight(&online_affinity) - 1, &online_affinity); } diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 3ea9fc8042..b579dfa307 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -194,9 +194,9 @@ int sched_init_vcpu(struct vcpu *v, unsigned int processor) */ v->processor = processor; if ( is_idle_domain(d) || d->is_pinned ) - cpumask_copy(v->cpu_affinity, cpumask_of(processor)); + cpumask_copy(v->cpu_hard_affinity, cpumask_of(processor)); else - cpumask_setall(v->cpu_affinity); + cpumask_setall(v->cpu_hard_affinity); /* Initialise the per-vcpu timers. */ init_timer(&v->periodic_timer, vcpu_periodic_timer_fn, @@ -285,7 +285,7 @@ int sched_move_domain(struct domain *d, struct cpupool *c) migrate_timer(&v->singleshot_timer, new_p); migrate_timer(&v->poll_timer, new_p); - cpumask_setall(v->cpu_affinity); + cpumask_setall(v->cpu_hard_affinity); lock = vcpu_schedule_lock_irq(v); v->processor = new_p; @@ -458,7 +458,7 @@ static void vcpu_migrate(struct vcpu *v) */ if ( pick_called && (new_lock == per_cpu(schedule_data, new_cpu).schedule_lock) && - cpumask_test_cpu(new_cpu, v->cpu_affinity) && + cpumask_test_cpu(new_cpu, v->cpu_hard_affinity) && cpumask_test_cpu(new_cpu, v->domain->cpupool->cpu_valid) ) break; @@ -561,7 +561,7 @@ void restore_vcpu_affinity(struct domain *d) if ( v->affinity_broken ) { printk(XENLOG_DEBUG "Restoring affinity for %pv\n", v); - cpumask_copy(v->cpu_affinity, v->cpu_affinity_saved); + cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved); v->affinity_broken = 0; } @@ -604,19 +604,20 @@ int cpu_disable_scheduler(unsigned int cpu) unsigned long flags; spinlock_t *lock = vcpu_schedule_lock_irqsave(v, &flags); - cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid); + cpumask_and(&online_affinity, v->cpu_hard_affinity, c->cpu_valid); if ( cpumask_empty(&online_affinity) && - cpumask_test_cpu(cpu, v->cpu_affinity) ) + cpumask_test_cpu(cpu, v->cpu_hard_affinity) ) { printk(XENLOG_DEBUG "Breaking affinity for %pv\n", v); if (system_state == SYS_STATE_suspend) { - cpumask_copy(v->cpu_affinity_saved, v->cpu_affinity); + cpumask_copy(v->cpu_hard_affinity_saved, + v->cpu_hard_affinity); v->affinity_broken = 1; } - cpumask_setall(v->cpu_affinity); + cpumask_setall(v->cpu_hard_affinity); } if ( v->processor == cpu ) @@ -664,7 +665,7 @@ int vcpu_set_affinity(struct vcpu *v, const cpumask_t *affinity) lock = vcpu_schedule_lock_irq(v); - cpumask_copy(v->cpu_affinity, affinity); + cpumask_copy(v->cpu_hard_affinity, affinity); /* Always ask the scheduler to re-evaluate placement * when changing the affinity */ diff --git a/xen/common/wait.c b/xen/common/wait.c index 3c9366c978..3f6ff410e2 100644 --- a/xen/common/wait.c +++ b/xen/common/wait.c @@ -134,7 +134,7 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv) /* Save current VCPU affinity; force wakeup on *this* CPU only. */ wqv->wakeup_cpu = smp_processor_id(); - cpumask_copy(&wqv->saved_affinity, curr->cpu_affinity); + cpumask_copy(&wqv->saved_affinity, curr->cpu_hard_affinity); if ( vcpu_set_affinity(curr, cpumask_of(wqv->wakeup_cpu)) ) { gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n"); @@ -183,7 +183,7 @@ void check_wakeup_from_wait(void) { /* Re-set VCPU affinity and re-enter the scheduler. */ struct vcpu *curr = current; - cpumask_copy(&wqv->saved_affinity, curr->cpu_affinity); + cpumask_copy(&wqv->saved_affinity, curr->cpu_hard_affinity); if ( vcpu_set_affinity(curr, cpumask_of(wqv->wakeup_cpu)) ) { gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n"); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 44851aee96..6f91abdfc8 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -219,11 +219,11 @@ struct vcpu spinlock_t virq_lock; /* Bitmask of CPUs on which this VCPU may run. */ - cpumask_var_t cpu_affinity; + cpumask_var_t cpu_hard_affinity; /* Used to change affinity temporarily. */ - cpumask_var_t cpu_affinity_tmp; + cpumask_var_t cpu_hard_affinity_tmp; /* Used to restore affinity across S3. */ - cpumask_var_t cpu_affinity_saved; + cpumask_var_t cpu_hard_affinity_saved; /* Bitmask of CPUs which are holding onto this VCPU's state. */ cpumask_var_t vcpu_dirty_cpumask; @@ -819,7 +819,7 @@ void watchdog_domain_destroy(struct domain *d); #define has_hvm_container_domain(d) ((d)->guest_type != guest_type_pv) #define has_hvm_container_vcpu(v) (has_hvm_container_domain((v)->domain)) #define is_pinned_vcpu(v) ((v)->domain->is_pinned || \ - cpumask_weight((v)->cpu_affinity) == 1) + cpumask_weight((v)->cpu_hard_affinity) == 1) #ifdef HAS_PASSTHROUGH #define need_iommu(d) ((d)->need_iommu) #else From db6b2215910327cd4401868c67a44680932c1f2a Mon Sep 17 00:00:00 2001 From: Dario Faggioli Date: Mon, 16 Jun 2014 12:12:28 +0200 Subject: [PATCH 7/9] sched: introduce soft-affinity and use it instead d->node-affinity Before this change, each vcpu had its own vcpu-affinity (in v->cpu_affinity), representing the set of pcpus where the vcpu is allowed to run. Since when NUMA-aware scheduling was introduced the (credit1 only, for now) scheduler also tries as much as it can to run all the vcpus of a domain on one of the nodes that constitutes the domain's node-affinity. The idea here is making the mechanism more general by: * allowing for this 'preference' for some pcpus/nodes to be expressed on a per-vcpu basis, instead than for the domain as a whole. That is to say, each vcpu should have its own set of preferred pcpus/nodes, instead than it being the very same for all the vcpus of the domain; * generalizing the idea of 'preferred pcpus' to not only NUMA awareness and support. That is to say, independently from it being or not (mostly) useful on NUMA systems, it should be possible to specify, for each vcpu, a set of pcpus where it prefers to run (in addition, and possibly unrelated to, the set of pcpus where it is allowed to run). We will be calling this set of *preferred* pcpus the vcpu's soft affinity, and this changes introduce it, and starts using it for scheduling, replacing the indirect use of the domain's NUMA node-affinity. This is more general, as soft affinity does not have to be related to NUMA. Nevertheless, it allows to achieve the same results of NUMA-aware scheduling, just by making soft affinity equal to the domain's node affinity, for all the vCPUs (e.g., from the toolstack). This also means renaming most of the NUMA-aware scheduling related functions, in credit1, to something more generic, hinting toward the concept of soft affinity rather than directly to NUMA awareness. As a side effects, this simplifies the code quit a bit. In fact, prior to this change, we needed to cache the translation of d->node_affinity (which is a nodemask_t) to a cpumask_t, since that is what scheduling decisions require (we used to keep it in node_affinity_cpumask). This, and all the complicated logic required to keep it updated, is not necessary any longer. The high level description of NUMA placement and scheduling in docs/misc/xl-numa-placement.markdown is being updated too, to match the new architecture. Signed-off-by: Dario Faggioli Reviewed-by: George Dunlap Acked-by: Jan Beulich --- docs/misc/xl-numa-placement.markdown | 144 ++++++++++++++++--------- xen/common/domain.c | 5 +- xen/common/keyhandler.c | 2 + xen/common/sched_credit.c | 153 +++++++++++---------------- xen/common/schedule.c | 8 +- xen/include/xen/sched-if.h | 2 - xen/include/xen/sched.h | 4 +- 7 files changed, 166 insertions(+), 152 deletions(-) diff --git a/docs/misc/xl-numa-placement.markdown b/docs/misc/xl-numa-placement.markdown index caa3fec288..9d64eaea08 100644 --- a/docs/misc/xl-numa-placement.markdown +++ b/docs/misc/xl-numa-placement.markdown @@ -12,13 +12,6 @@ is quite more complex and slow. On these machines, a NUMA node is usually defined as a set of processor cores (typically a physical CPU package) and the memory directly attached to the set of cores. -The Xen hypervisor deals with NUMA machines by assigning to each domain -a "node affinity", i.e., a set of NUMA nodes of the host from which they -get their memory allocated. Also, even if the node affinity of a domain -is allowed to change on-line, it is very important to "place" the domain -correctly when it is fist created, as the most of its memory is allocated -at that time and can not (for now) be moved easily. - NUMA awareness becomes very important as soon as many domains start running memory-intensive workloads on a shared host. In fact, the cost of accessing non node-local memory locations is very high, and the @@ -27,14 +20,37 @@ performance degradation is likely to be noticeable. For more information, have a look at the [Xen NUMA Introduction][numa_intro] page on the Wiki. +## Xen and NUMA machines: the concept of _node-affinity_ ## + +The Xen hypervisor deals with NUMA machines throughout the concept of +_node-affinity_. The node-affinity of a domain is the set of NUMA nodes +of the host where the memory for the domain is being allocated (mostly, +at domain creation time). This is, at least in principle, different and +unrelated with the vCPU (hard and soft, see below) scheduling affinity, +which instead is the set of pCPUs where the vCPU is allowed (or prefers) +to run. + +Of course, despite the fact that they belong to and affect different +subsystems, the domain node-affinity and the vCPUs affinity are not +completely independent. +In fact, if the domain node-affinity is not explicitly specified by the +user, via the proper libxl calls or xl config item, it will be computed +basing on the vCPUs' scheduling affinity. + +Notice that, even if the node affinity of a domain may change on-line, +it is very important to "place" the domain correctly when it is fist +created, as the most of its memory is allocated at that time and can +not (for now) be moved easily. + ### Placing via pinning and cpupools ### -The simplest way of placing a domain on a NUMA node is statically pinning -the domain's vCPUs to the pCPUs of the node. This goes under the name of -CPU affinity and can be set through the "cpus=" option in the config file -(more about this below). Another option is to pool together the pCPUs -spanning the node and put the domain in such a cpupool with the "pool=" -config option (as documented in our [Wiki][cpupools_howto]). +The simplest way of placing a domain on a NUMA node is setting the hard +scheduling affinity of the domain's vCPUs to the pCPUs of the node. This +also goes under the name of vCPU pinning, and can be done through the +"cpus=" option in the config file (more about this below). Another option +is to pool together the pCPUs spanning the node and put the domain in +such a _cpupool_ with the "pool=" config option (as documented in our +[Wiki][cpupools_howto]). In both the above cases, the domain will not be able to execute outside the specified set of pCPUs for any reasons, even if all those pCPUs are @@ -45,24 +61,45 @@ may come at he cost of some load imbalances. ### NUMA aware scheduling ### -If the credit scheduler is in use, the concept of node affinity defined -above does not only apply to memory. In fact, starting from Xen 4.3, the -scheduler always tries to run the domain's vCPUs on one of the nodes in -its node affinity. Only if that turns out to be impossible, it will just -pick any free pCPU. +If using the credit1 scheduler, and starting from Xen 4.3, the scheduler +itself always tries to run the domain's vCPUs on one of the nodes in +its node-affinity. Only if that turns out to be impossible, it will just +pick any free pCPU. Locality of access is less guaranteed than in the +pinning case, but that comes along with better chances to exploit all +the host resources (e.g., the pCPUs). -This is, therefore, something more flexible than CPU affinity, as a domain -can still run everywhere, it just prefers some nodes rather than others. -Locality of access is less guaranteed than in the pinning case, but that -comes along with better chances to exploit all the host resources (e.g., -the pCPUs). +Starting from Xen 4.5, credit1 supports two forms of affinity: hard and +soft, both on a per-vCPU basis. This means each vCPU can have its own +soft affinity, stating where such vCPU prefers to execute on. This is +less strict than what it (also starting from 4.5) is called hard affinity, +as the vCPU can potentially run everywhere, it just prefers some pCPUs +rather than others. +In Xen 4.5, therefore, NUMA-aware scheduling is achieved by matching the +soft affinity of the vCPUs of a domain with its node-affinity. -In fact, if all the pCPUs in a domain's node affinity are busy, it is -possible for the domain to run outside of there, but it is very likely that -slower execution (due to remote memory accesses) is still better than no -execution at all, as it would happen with pinning. For this reason, NUMA -aware scheduling has the potential of bringing substantial performances -benefits, although this will depend on the workload. +In fact, as it was for 4.3, if all the pCPUs in a vCPU's soft affinity +are busy, it is possible for the domain to run outside from there. The +idea is that slower execution (due to remote memory accesses) is still +better than no execution at all (as it would happen with pinning). For +this reason, NUMA aware scheduling has the potential of bringing +substantial performances benefits, although this will depend on the +workload. + +Notice that, for each vCPU, the following three scenarios are possbile: + + * a vCPU *is pinned* to some pCPUs and *does not have* any soft affinity + In this case, the vCPU is always scheduled on one of the pCPUs to which + it is pinned, without any specific peference among them. + * a vCPU *has* its own soft affinity and *is not* pinned to any particular + pCPU. In this case, the vCPU can run on every pCPU. Nevertheless, the + scheduler will try to have it running on one of the pCPUs in its soft + affinity; + * a vCPU *has* its own vCPU soft affinity and *is also* pinned to some + pCPUs. In this case, the vCPU is always scheduled on one of the pCPUs + onto which it is pinned, with, among them, a preference for the ones + that also forms its soft affinity. In case pinning and soft affinity + form two disjoint sets of pCPUs, pinning "wins", and the soft affinity + is just ignored. ## Guest placement in xl ## @@ -71,25 +108,23 @@ both manual or automatic placement of them across the host's NUMA nodes. Note that xm/xend does a very similar thing, the only differences being the details of the heuristics adopted for automatic placement (see below), -and the lack of support (in both xm/xend and the Xen versions where that\ +and the lack of support (in both xm/xend and the Xen versions where that was the default toolstack) for NUMA aware scheduling. ### Placing the guest manually ### Thanks to the "cpus=" option, it is possible to specify where a domain should be created and scheduled on, directly in its config file. This -affects NUMA placement and memory accesses as the hypervisor constructs -the node affinity of a VM basing right on its CPU affinity when it is -created. +affects NUMA placement and memory accesses as, in this case, the +hypervisor constructs the node-affinity of a VM basing right on its +vCPU pinning when it is created. This is very simple and effective, but requires the user/system -administrator to explicitly specify affinities for each and every domain, +administrator to explicitly specify the pinning for each and every domain, or Xen won't be able to guarantee the locality for their memory accesses. -Notice that this also pins the domain's vCPUs to the specified set of -pCPUs, so it not only sets the domain's node affinity (its memory will -come from the nodes to which the pCPUs belong), but at the same time -forces the vCPUs of the domain to be scheduled on those same pCPUs. +That, of course, also mean the vCPUs of the domain will only be able to +execute on those same pCPUs. ### Placing the guest automatically ### @@ -97,7 +132,9 @@ If no "cpus=" option is specified in the config file, libxl tries to figure out on its own on which node(s) the domain could fit best. If it finds one (some), the domain's node affinity get set to there, and both memory allocations and NUMA aware scheduling (for the credit -scheduler and starting from Xen 4.3) will comply with it. +scheduler and starting from Xen 4.3) will comply with it. Starting from +Xen 4.5, this also means that the mask resulting from this "fitting" +procedure will become the soft affinity of all the vCPUs of the domain. It is worthwhile noting that optimally fitting a set of VMs on the NUMA nodes of an host is an incarnation of the Bin Packing Problem. In fact, @@ -142,34 +179,43 @@ any placement from happening: libxl_defbool_set(&domain_build_info->numa_placement, false); -Also, if `numa_placement` is set to `true`, the domain must not -have any CPU affinity (i.e., `domain_build_info->cpumap` must -have all its bits set, as it is by default), or domain creation -will fail returning `ERROR_INVAL`. +Also, if `numa_placement` is set to `true`, the domain's vCPUs must +not be pinned (i.e., `domain_build_info->cpumap` must have all its +bits set, as it is by default), or domain creation will fail with +`ERROR_INVAL`. Starting from Xen 4.3, in case automatic placement happens (and is -successful), it will affect the domain's node affinity and _not_ its -CPU affinity. Namely, the domain's vCPUs will not be pinned to any +successful), it will affect the domain's node-affinity and _not_ its +vCPU pinning. Namely, the domain's vCPUs will not be pinned to any pCPU on the host, but the memory from the domain will come from the selected node(s) and the NUMA aware scheduling (if the credit scheduler -is in use) will try to keep the domain there as much as possible. +is in use) will try to keep the domain's vCPUs there as much as possible. Besides than that, looking and/or tweaking the placement algorithm search "Automatic NUMA placement" in libxl\_internal.h. Note this may change in future versions of Xen/libxl. +## Xen < 4.5 ## + +The concept of vCPU soft affinity has been introduced for the first time +in Xen 4.5. In 4.3, it is the domain's node-affinity that drives the +NUMA-aware scheduler. The main difference is soft affinity is per-vCPU, +and so each vCPU can have its own mask of pCPUs, while node-affinity is +per-domain, that is the equivalent of having all the vCPUs with the same +soft affinity. + ## Xen < 4.3 ## As NUMA aware scheduling is a new feature of Xen 4.3, things are a little bit different for earlier version of Xen. If no "cpus=" option is specified and Xen 4.2 is in use, the automatic placement algorithm still runs, but the results is used to _pin_ the vCPUs of the domain to the output node(s). -This is consistent with what was happening with xm/xend, which were also -affecting the domain's CPU affinity. +This is consistent with what was happening with xm/xend. On a version of Xen earlier than 4.2, there is not automatic placement at -all in xl or libxl, and hence no node or CPU affinity being affected. +all in xl or libxl, and hence no node-affinity, vCPU affinity or pinning +being introduced/modified. ## Limitations ## diff --git a/xen/common/domain.c b/xen/common/domain.c index 141a5dc2e4..e20d3bff1d 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -128,6 +128,7 @@ struct vcpu *alloc_vcpu( if ( !zalloc_cpumask_var(&v->cpu_hard_affinity) || !zalloc_cpumask_var(&v->cpu_hard_affinity_tmp) || !zalloc_cpumask_var(&v->cpu_hard_affinity_saved) || + !zalloc_cpumask_var(&v->cpu_soft_affinity) || !zalloc_cpumask_var(&v->vcpu_dirty_cpumask) ) goto fail_free; @@ -159,6 +160,7 @@ struct vcpu *alloc_vcpu( free_cpumask_var(v->cpu_hard_affinity); free_cpumask_var(v->cpu_hard_affinity_tmp); free_cpumask_var(v->cpu_hard_affinity_saved); + free_cpumask_var(v->cpu_soft_affinity); free_cpumask_var(v->vcpu_dirty_cpumask); free_vcpu_struct(v); return NULL; @@ -446,8 +448,6 @@ void domain_update_node_affinity(struct domain *d) node_set(node, d->node_affinity); } - sched_set_node_affinity(d, &d->node_affinity); - spin_unlock(&d->node_affinity_lock); free_cpumask_var(online_affinity); @@ -795,6 +795,7 @@ static void complete_domain_destroy(struct rcu_head *head) free_cpumask_var(v->cpu_hard_affinity); free_cpumask_var(v->cpu_hard_affinity_tmp); free_cpumask_var(v->cpu_hard_affinity_saved); + free_cpumask_var(v->cpu_soft_affinity); free_cpumask_var(v->vcpu_dirty_cpumask); free_vcpu_struct(v); } diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c index d6eb0263dc..809378c2a5 100644 --- a/xen/common/keyhandler.c +++ b/xen/common/keyhandler.c @@ -297,6 +297,8 @@ static void dump_domains(unsigned char key) printk("dirty_cpus=%s ", tmpstr); cpuset_print(tmpstr, sizeof(tmpstr), v->cpu_hard_affinity); printk("cpu_affinity=%s\n", tmpstr); + cpuset_print(tmpstr, sizeof(tmpstr), v->cpu_soft_affinity); + printk("cpu_soft_affinity=%s\n", tmpstr); printk(" pause_count=%d pause_flags=%lx\n", atomic_read(&v->pause_count), v->pause_flags); arch_dump_vcpu_info(v); diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index c6a2560abe..8b02b7b192 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -112,10 +112,24 @@ /* - * Node Balancing + * Hard and soft affinity load balancing. + * + * Idea is each vcpu has some pcpus that it prefers, some that it does not + * prefer but is OK with, and some that it cannot run on at all. The first + * set of pcpus are the ones that are both in the soft affinity *and* in the + * hard affinity; the second set of pcpus are the ones that are in the hard + * affinity but *not* in the soft affinity; the third set of pcpus are the + * ones that are not in the hard affinity. + * + * We implement a two step balancing logic. Basically, every time there is + * the need to decide where to run a vcpu, we first check the soft affinity + * (well, actually, the && between soft and hard affinity), to see if we can + * send it where it prefers to (and can) run on. However, if the first step + * does not find any suitable and free pcpu, we fall back checking the hard + * affinity. */ -#define CSCHED_BALANCE_NODE_AFFINITY 0 -#define CSCHED_BALANCE_CPU_AFFINITY 1 +#define CSCHED_BALANCE_SOFT_AFFINITY 0 +#define CSCHED_BALANCE_HARD_AFFINITY 1 /* * Boot parameters @@ -138,7 +152,7 @@ struct csched_pcpu { /* * Convenience macro for accessing the per-PCPU cpumask we need for - * implementing the two steps (vcpu and node affinity) balancing logic. + * implementing the two steps (soft and hard affinity) balancing logic. * It is stored in csched_pcpu so that serialization is not an issue, * as there is a csched_pcpu for each PCPU and we always hold the * runqueue spin-lock when using this. @@ -178,9 +192,6 @@ struct csched_dom { struct list_head active_vcpu; struct list_head active_sdom_elem; struct domain *dom; - /* cpumask translated from the domain's node-affinity. - * Basically, the CPUs we prefer to be scheduled on. */ - cpumask_var_t node_affinity_cpumask; uint16_t active_vcpu_count; uint16_t weight; uint16_t cap; @@ -261,59 +272,28 @@ __runq_remove(struct csched_vcpu *svc) list_del_init(&svc->runq_elem); } -/* - * Translates node-affinity mask into a cpumask, so that we can use it during - * actual scheduling. That of course will contain all the cpus from all the - * set nodes in the original node-affinity mask. - * - * Note that any serialization needed to access mask safely is complete - * responsibility of the caller of this function/hook. - */ -static void csched_set_node_affinity( - const struct scheduler *ops, - struct domain *d, - nodemask_t *mask) -{ - struct csched_dom *sdom; - int node; - - /* Skip idle domain since it doesn't even have a node_affinity_cpumask */ - if ( unlikely(is_idle_domain(d)) ) - return; - - sdom = CSCHED_DOM(d); - cpumask_clear(sdom->node_affinity_cpumask); - for_each_node_mask( node, *mask ) - cpumask_or(sdom->node_affinity_cpumask, sdom->node_affinity_cpumask, - &node_to_cpumask(node)); -} #define for_each_csched_balance_step(step) \ - for ( (step) = 0; (step) <= CSCHED_BALANCE_CPU_AFFINITY; (step)++ ) + for ( (step) = 0; (step) <= CSCHED_BALANCE_HARD_AFFINITY; (step)++ ) /* - * vcpu-affinity balancing is always necessary and must never be skipped. - * OTOH, if a domain's node-affinity is said to be automatically computed - * (or if it just spans all the nodes), we can safely avoid dealing with - * node-affinity entirely. + * Hard affinity balancing is always necessary and must never be skipped. + * OTOH, if the vcpu's soft affinity is full (it spans all the possible + * pcpus) we can safely avoid dealing with it entirely. * - * Node-affinity is also deemed meaningless in case it has empty - * intersection with mask, to cover the cases where using the node-affinity + * A vcpu's soft affinity is also deemed meaningless in case it has empty + * intersection with mask, to cover the cases where using the soft affinity * mask seems legit, but would instead led to trying to schedule the vcpu * on _no_ pcpu! Typical use cases are for mask to be equal to the vcpu's - * vcpu-affinity, or to the && of vcpu-affinity and the set of online cpus + * hard affinity, or to the && of hard affinity and the set of online cpus * in the domain's cpupool. */ -static inline int __vcpu_has_node_affinity(const struct vcpu *vc, +static inline int __vcpu_has_soft_affinity(const struct vcpu *vc, const cpumask_t *mask) { - const struct domain *d = vc->domain; - const struct csched_dom *sdom = CSCHED_DOM(d); - - if ( d->auto_node_affinity - || cpumask_full(sdom->node_affinity_cpumask) - || !cpumask_intersects(sdom->node_affinity_cpumask, mask) ) + if ( cpumask_full(vc->cpu_soft_affinity) + || !cpumask_intersects(vc->cpu_soft_affinity, mask) ) return 0; return 1; @@ -321,23 +301,22 @@ static inline int __vcpu_has_node_affinity(const struct vcpu *vc, /* * Each csched-balance step uses its own cpumask. This function determines - * which one (given the step) and copies it in mask. For the node-affinity - * balancing step, the pcpus that are not part of vc's vcpu-affinity are + * which one (given the step) and copies it in mask. For the soft affinity + * balancing step, the pcpus that are not part of vc's hard affinity are * filtered out from the result, to avoid running a vcpu where it would * like, but is not allowed to! */ static void csched_balance_cpumask(const struct vcpu *vc, int step, cpumask_t *mask) { - if ( step == CSCHED_BALANCE_NODE_AFFINITY ) + if ( step == CSCHED_BALANCE_SOFT_AFFINITY ) { - cpumask_and(mask, CSCHED_DOM(vc->domain)->node_affinity_cpumask, - vc->cpu_hard_affinity); + cpumask_and(mask, vc->cpu_soft_affinity, vc->cpu_hard_affinity); if ( unlikely(cpumask_empty(mask)) ) cpumask_copy(mask, vc->cpu_hard_affinity); } - else /* step == CSCHED_BALANCE_CPU_AFFINITY */ + else /* step == CSCHED_BALANCE_HARD_AFFINITY */ cpumask_copy(mask, vc->cpu_hard_affinity); } @@ -398,15 +377,15 @@ __runq_tickle(unsigned int cpu, struct csched_vcpu *new) else if ( !idlers_empty ) { /* - * Node and vcpu-affinity balancing loop. For vcpus without - * a useful node-affinity, consider vcpu-affinity only. + * Soft and hard affinity balancing loop. For vcpus without + * a useful soft affinity, consider hard affinity only. */ for_each_csched_balance_step( balance_step ) { int new_idlers_empty; - if ( balance_step == CSCHED_BALANCE_NODE_AFFINITY - && !__vcpu_has_node_affinity(new->vcpu, + if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY + && !__vcpu_has_soft_affinity(new->vcpu, new->vcpu->cpu_hard_affinity) ) continue; @@ -418,11 +397,11 @@ __runq_tickle(unsigned int cpu, struct csched_vcpu *new) /* * Let's not be too harsh! If there aren't idlers suitable - * for new in its node-affinity mask, make sure we check its - * vcpu-affinity as well, before taking final decisions. + * for new in its soft affinity mask, make sure we check its + * hard affinity as well, before taking final decisions. */ if ( new_idlers_empty - && balance_step == CSCHED_BALANCE_NODE_AFFINITY ) + && balance_step == CSCHED_BALANCE_SOFT_AFFINITY ) continue; /* @@ -649,23 +628,23 @@ _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc, bool_t commit) /* * We want to pick up a pcpu among the ones that are online and * can accommodate vc, which is basically what we computed above - * and stored in cpus. As far as vcpu-affinity is concerned, + * and stored in cpus. As far as hard affinity is concerned, * there always will be at least one of these pcpus, hence cpus * is never empty and the calls to cpumask_cycle() and * cpumask_test_cpu() below are ok. * - * On the other hand, when considering node-affinity too, it + * On the other hand, when considering soft affinity too, it * is possible for the mask to become empty (for instance, if the * domain has been put in a cpupool that does not contain any of the - * nodes in its node-affinity), which would result in the ASSERT()-s + * pcpus in its soft affinity), which would result in the ASSERT()-s * inside cpumask_*() operations triggering (in debug builds). * - * Therefore, in this case, we filter the node-affinity mask against - * cpus and, if the result is empty, we just skip the node-affinity + * Therefore, in this case, we filter the soft affinity mask against + * cpus and, if the result is empty, we just skip the soft affinity * balancing step all together. */ - if ( balance_step == CSCHED_BALANCE_NODE_AFFINITY - && !__vcpu_has_node_affinity(vc, &cpus) ) + if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY + && !__vcpu_has_soft_affinity(vc, &cpus) ) continue; /* Pick an online CPU from the proper affinity mask */ @@ -1122,13 +1101,6 @@ csched_alloc_domdata(const struct scheduler *ops, struct domain *dom) if ( sdom == NULL ) return NULL; - if ( !alloc_cpumask_var(&sdom->node_affinity_cpumask) ) - { - xfree(sdom); - return NULL; - } - cpumask_setall(sdom->node_affinity_cpumask); - /* Initialize credit and weight */ INIT_LIST_HEAD(&sdom->active_vcpu); INIT_LIST_HEAD(&sdom->active_sdom_elem); @@ -1158,9 +1130,6 @@ csched_dom_init(const struct scheduler *ops, struct domain *dom) static void csched_free_domdata(const struct scheduler *ops, void *data) { - struct csched_dom *sdom = data; - - free_cpumask_var(sdom->node_affinity_cpumask); xfree(data); } @@ -1486,19 +1455,19 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step) BUG_ON( is_idle_vcpu(vc) ); /* - * If the vcpu has no useful node-affinity, skip this vcpu. - * In fact, what we want is to check if we have any node-affine - * work to steal, before starting to look at vcpu-affine work. + * If the vcpu has no useful soft affinity, skip this vcpu. + * In fact, what we want is to check if we have any "soft-affine + * work" to steal, before starting to look at "hard-affine work". * * Notice that, if not even one vCPU on this runq has a useful - * node-affinity, we could have avoid considering this runq for - * a node balancing step in the first place. This, for instance, + * soft affinity, we could have avoid considering this runq for + * a soft balancing step in the first place. This, for instance, * can be implemented by taking note of on what runq there are - * vCPUs with useful node-affinities in some sort of bitmap + * vCPUs with useful soft affinities in some sort of bitmap * or counter. */ - if ( balance_step == CSCHED_BALANCE_NODE_AFFINITY - && !__vcpu_has_node_affinity(vc, vc->cpu_hard_affinity) ) + if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY + && !__vcpu_has_soft_affinity(vc, vc->cpu_hard_affinity) ) continue; csched_balance_cpumask(vc, balance_step, csched_balance_mask); @@ -1546,17 +1515,17 @@ csched_load_balance(struct csched_private *prv, int cpu, SCHED_STAT_CRANK(load_balance_other); /* - * Let's look around for work to steal, taking both vcpu-affinity - * and node-affinity into account. More specifically, we check all + * Let's look around for work to steal, taking both hard affinity + * and soft affinity into account. More specifically, we check all * the non-idle CPUs' runq, looking for: - * 1. any node-affine work to steal first, - * 2. if not finding anything, any vcpu-affine work to steal. + * 1. any "soft-affine work" to steal first, + * 2. if not finding anything, any "hard-affine work" to steal. */ for_each_csched_balance_step( bstep ) { /* * We peek at the non-idling CPUs in a node-wise fashion. In fact, - * it is more likely that we find some node-affine work on our same + * it is more likely that we find some affine work on our same * node, not to mention that migrating vcpus within the same node * could well expected to be cheaper than across-nodes (memory * stays local, there might be some node-wide cache[s], etc.). @@ -1982,8 +1951,6 @@ const struct scheduler sched_credit_def = { .adjust = csched_dom_cntl, .adjust_global = csched_sys_cntl, - .set_node_affinity = csched_set_node_affinity, - .pick_cpu = csched_cpu_pick, .do_schedule = csched_schedule, diff --git a/xen/common/schedule.c b/xen/common/schedule.c index b579dfa307..d76a425952 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -198,6 +198,8 @@ int sched_init_vcpu(struct vcpu *v, unsigned int processor) else cpumask_setall(v->cpu_hard_affinity); + cpumask_setall(v->cpu_soft_affinity); + /* Initialise the per-vcpu timers. */ init_timer(&v->periodic_timer, vcpu_periodic_timer_fn, v, v->processor); @@ -286,6 +288,7 @@ int sched_move_domain(struct domain *d, struct cpupool *c) migrate_timer(&v->poll_timer, new_p); cpumask_setall(v->cpu_hard_affinity); + cpumask_setall(v->cpu_soft_affinity); lock = vcpu_schedule_lock_irq(v); v->processor = new_p; @@ -645,11 +648,6 @@ int cpu_disable_scheduler(unsigned int cpu) return ret; } -void sched_set_node_affinity(struct domain *d, nodemask_t *mask) -{ - SCHED_OP(DOM2OP(d), set_node_affinity, d, mask); -} - int vcpu_set_affinity(struct vcpu *v, const cpumask_t *affinity) { cpumask_t online_affinity; diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h index d95e254c5a..4164dffb3e 100644 --- a/xen/include/xen/sched-if.h +++ b/xen/include/xen/sched-if.h @@ -158,8 +158,6 @@ struct scheduler { struct xen_domctl_scheduler_op *); int (*adjust_global) (const struct scheduler *, struct xen_sysctl_scheduler_op *); - void (*set_node_affinity) (const struct scheduler *, - struct domain *, nodemask_t *); void (*dump_settings) (const struct scheduler *); void (*dump_cpu_state) (const struct scheduler *, int); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 6f91abdfc8..445b659dd2 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -225,6 +225,9 @@ struct vcpu /* Used to restore affinity across S3. */ cpumask_var_t cpu_hard_affinity_saved; + /* Bitmask of CPUs on which this VCPU prefers to run. */ + cpumask_var_t cpu_soft_affinity; + /* Bitmask of CPUs which are holding onto this VCPU's state. */ cpumask_var_t vcpu_dirty_cpumask; @@ -627,7 +630,6 @@ void sched_destroy_domain(struct domain *d); int sched_move_domain(struct domain *d, struct cpupool *c); long sched_adjust(struct domain *, struct xen_domctl_scheduler_op *); long sched_adjust_global(struct xen_sysctl_scheduler_op *); -void sched_set_node_affinity(struct domain *, nodemask_t *); int sched_id(void); void sched_tick_suspend(void); void sched_tick_resume(void); From bf508bd2fb83b51f8547d1969629e05b6b87d1c5 Mon Sep 17 00:00:00 2001 From: Dario Faggioli Date: Mon, 16 Jun 2014 12:13:03 +0200 Subject: [PATCH 8/9] derive NUMA node affinity from hard and soft CPU affinity if a domain's NUMA node-affinity (which is what controls memory allocations) is provided by the user/toolstack, it just is not touched. However, if the user does not say anything, leaving it all to Xen, let's compute it in the following way: 1. cpupool's cpus & hard-affinity & soft-affinity 2. if (1) is empty: cpupool's cpus & hard-affinity This guarantees memory to be allocated from the narrowest possible set of NUMA nodes, ad makes it relatively easy to set up NUMA-aware scheduling on top of soft affinity. Note that such 'narrowest set' is guaranteed to be non-empty. Signed-off-by: Dario Faggioli Acked-by: George Dunlap Acked-by: Jan Beulich --- xen/common/domain.c | 61 +++++++++++++++++++++++++++---------------- xen/common/schedule.c | 4 ++- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/xen/common/domain.c b/xen/common/domain.c index e20d3bff1d..c3a576e2e4 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -409,17 +409,17 @@ struct domain *domain_create( void domain_update_node_affinity(struct domain *d) { - cpumask_var_t cpumask; - cpumask_var_t online_affinity; + cpumask_var_t dom_cpumask, dom_cpumask_soft; + cpumask_t *dom_affinity; const cpumask_t *online; struct vcpu *v; - unsigned int node; + unsigned int cpu; - if ( !zalloc_cpumask_var(&cpumask) ) + if ( !zalloc_cpumask_var(&dom_cpumask) ) return; - if ( !alloc_cpumask_var(&online_affinity) ) + if ( !zalloc_cpumask_var(&dom_cpumask_soft) ) { - free_cpumask_var(cpumask); + free_cpumask_var(dom_cpumask); return; } @@ -427,31 +427,48 @@ void domain_update_node_affinity(struct domain *d) spin_lock(&d->node_affinity_lock); - for_each_vcpu ( d, v ) - { - cpumask_and(online_affinity, v->cpu_hard_affinity, online); - cpumask_or(cpumask, cpumask, online_affinity); - } - /* - * If d->auto_node_affinity is true, the domain's node-affinity mask - * (d->node_affinity) is automaically computed from all the domain's - * vcpus' vcpu-affinity masks (the union of which we have just built - * above in cpumask). OTOH, if d->auto_node_affinity is false, we - * must leave the node-affinity of the domain alone. + * If d->auto_node_affinity is true, let's compute the domain's + * node-affinity and update d->node_affinity accordingly. if false, + * just leave d->auto_node_affinity alone. */ if ( d->auto_node_affinity ) { + /* + * We want the narrowest possible set of pcpus (to get the narowest + * possible set of nodes). What we need is the cpumask of where the + * domain can run (the union of the hard affinity of all its vcpus), + * and the full mask of where it would prefer to run (the union of + * the soft affinity of all its various vcpus). Let's build them. + */ + for_each_vcpu ( d, v ) + { + cpumask_or(dom_cpumask, dom_cpumask, v->cpu_hard_affinity); + cpumask_or(dom_cpumask_soft, dom_cpumask_soft, + v->cpu_soft_affinity); + } + /* Filter out non-online cpus */ + cpumask_and(dom_cpumask, dom_cpumask, online); + ASSERT(!cpumask_empty(dom_cpumask)); + /* And compute the intersection between hard, online and soft */ + cpumask_and(dom_cpumask_soft, dom_cpumask_soft, dom_cpumask); + + /* + * If not empty, the intersection of hard, soft and online is the + * narrowest set we want. If empty, we fall back to hard&online. + */ + dom_affinity = cpumask_empty(dom_cpumask_soft) ? + dom_cpumask : dom_cpumask_soft; + nodes_clear(d->node_affinity); - for_each_online_node ( node ) - if ( cpumask_intersects(&node_to_cpumask(node), cpumask) ) - node_set(node, d->node_affinity); + for_each_cpu ( cpu, dom_affinity ) + node_set(cpu_to_node(cpu), d->node_affinity); } spin_unlock(&d->node_affinity_lock); - free_cpumask_var(online_affinity); - free_cpumask_var(cpumask); + free_cpumask_var(dom_cpumask_soft); + free_cpumask_var(dom_cpumask); } diff --git a/xen/common/schedule.c b/xen/common/schedule.c index d76a425952..e57cd9159a 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -310,7 +310,9 @@ int sched_move_domain(struct domain *d, struct cpupool *c) SCHED_OP(old_ops, free_vdata, vcpudata); } - domain_update_node_affinity(d); + /* Do we have vcpus already? If not, no need to update node-affinity */ + if ( d->vcpu ) + domain_update_node_affinity(d); domain_unpause(d); From 6e4ecc6d5884b816ed6eb1949561fdfb85acd179 Mon Sep 17 00:00:00 2001 From: Dario Faggioli Date: Mon, 16 Jun 2014 12:13:25 +0200 Subject: [PATCH 9/9] sched: DOMCTL_*vcpuaffinity works with hard and soft affinity by adding a flag for the caller to specify which one he cares about. At the same time, enable the caller to get back the "effective affinity" of the vCPU. That is the intersection between cpupool's cpus, the (new) hard affinity and, for soft affinity, the (new) soft affinity. In fact, despite what has been successfully set with the DOMCTL_setvcpuaffinity hypercall, the Xen scheduler will never run a vCPU outside of its hard affinity or of its domain's cpupool. This happens by adding another cpumap to the interface and making both the cpumaps IN/OUT parameters (for DOMCTL_setvcpuaffinity, they're of course out-only for DOMCTL_getvcpuaffinity). Signed-off-by: Dario Faggioli Reviewed-by: George Dunlap Acked-by: Jan Beulich Acked-by: Ian Campbell --- tools/libxc/xc_domain.c | 12 ++-- xen/arch/x86/traps.c | 4 +- xen/common/domctl.c | 107 ++++++++++++++++++++++++++++++++---- xen/common/schedule.c | 35 ++++++++---- xen/common/wait.c | 6 +- xen/include/public/domctl.h | 29 +++++++++- xen/include/xen/sched.h | 3 +- 7 files changed, 162 insertions(+), 34 deletions(-) diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index 9c8653e79a..26edbaf098 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -225,13 +225,14 @@ int xc_vcpu_setaffinity(xc_interface *xch, domctl.cmd = XEN_DOMCTL_setvcpuaffinity; domctl.domain = (domid_t)domid; - domctl.u.vcpuaffinity.vcpu = vcpu; + domctl.u.vcpuaffinity.vcpu = vcpu; + domctl.u.vcpuaffinity.flags = XEN_VCPUAFFINITY_HARD; memcpy(local, cpumap, cpusize); - set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap.bitmap, local); + set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap_hard.bitmap, local); - domctl.u.vcpuaffinity.cpumap.nr_bits = cpusize * 8; + domctl.u.vcpuaffinity.cpumap_hard.nr_bits = cpusize * 8; ret = do_domctl(xch, &domctl); @@ -269,9 +270,10 @@ int xc_vcpu_getaffinity(xc_interface *xch, domctl.cmd = XEN_DOMCTL_getvcpuaffinity; domctl.domain = (domid_t)domid; domctl.u.vcpuaffinity.vcpu = vcpu; + domctl.u.vcpuaffinity.flags = XEN_VCPUAFFINITY_HARD; - set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap.bitmap, local); - domctl.u.vcpuaffinity.cpumap.nr_bits = cpusize * 8; + set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap_hard.bitmap, local); + domctl.u.vcpuaffinity.cpumap_hard.nr_bits = cpusize * 8; ret = do_domctl(xch, &domctl); diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index cc1c87cff0..5897928fd3 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -3183,7 +3183,7 @@ static void nmi_mce_softirq(void) * Make sure to wakeup the vcpu on the * specified processor. */ - vcpu_set_affinity(st->vcpu, cpumask_of(st->processor)); + vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor)); /* Affinity is restored in the iret hypercall. */ } @@ -3212,7 +3212,7 @@ void async_exception_cleanup(struct vcpu *curr) if ( !cpumask_empty(curr->cpu_hard_affinity_tmp) && !cpumask_equal(curr->cpu_hard_affinity_tmp, curr->cpu_hard_affinity) ) { - vcpu_set_affinity(curr, curr->cpu_hard_affinity_tmp); + vcpu_set_hard_affinity(curr, curr->cpu_hard_affinity_tmp); cpumask_clear(curr->cpu_hard_affinity_tmp); } diff --git a/xen/common/domctl.c b/xen/common/domctl.c index b5c5c6ce3c..000993f70e 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -287,6 +287,16 @@ void domctl_lock_release(void) spin_unlock(¤t->domain->hypercall_deadlock_mutex); } +static inline +int vcpuaffinity_params_invalid(const xen_domctl_vcpuaffinity_t *vcpuaff) +{ + return vcpuaff->flags == 0 || + ((vcpuaff->flags & XEN_VCPUAFFINITY_HARD) && + guest_handle_is_null(vcpuaff->cpumap_hard.bitmap)) || + ((vcpuaff->flags & XEN_VCPUAFFINITY_SOFT) && + guest_handle_is_null(vcpuaff->cpumap_soft.bitmap)); +} + long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) { long ret = 0; @@ -601,31 +611,108 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) case XEN_DOMCTL_getvcpuaffinity: { struct vcpu *v; + xen_domctl_vcpuaffinity_t *vcpuaff = &op->u.vcpuaffinity; ret = -EINVAL; - if ( op->u.vcpuaffinity.vcpu >= d->max_vcpus ) + if ( vcpuaff->vcpu >= d->max_vcpus ) break; ret = -ESRCH; - if ( (v = d->vcpu[op->u.vcpuaffinity.vcpu]) == NULL ) + if ( (v = d->vcpu[vcpuaff->vcpu]) == NULL ) + break; + + ret = -EINVAL; + if ( vcpuaffinity_params_invalid(vcpuaff) ) break; if ( op->cmd == XEN_DOMCTL_setvcpuaffinity ) { - cpumask_var_t new_affinity; + cpumask_var_t new_affinity, old_affinity; + cpumask_t *online = cpupool_online_cpumask(v->domain->cpupool);; - ret = xenctl_bitmap_to_cpumask( - &new_affinity, &op->u.vcpuaffinity.cpumap); - if ( !ret ) + /* + * We want to be able to restore hard affinity if we are trying + * setting both and changing soft affinity (which happens later, + * when hard affinity has been succesfully chaged already) fails. + */ + if ( !alloc_cpumask_var(&old_affinity) ) { - ret = vcpu_set_affinity(v, new_affinity); - free_cpumask_var(new_affinity); + ret = -ENOMEM; + break; } + cpumask_copy(old_affinity, v->cpu_hard_affinity); + + if ( !alloc_cpumask_var(&new_affinity) ) + { + free_cpumask_var(old_affinity); + ret = -ENOMEM; + break; + } + + /* + * We both set a new affinity and report back to the caller what + * the scheduler will be effectively using. + */ + if ( vcpuaff->flags & XEN_VCPUAFFINITY_HARD ) + { + ret = xenctl_bitmap_to_bitmap(cpumask_bits(new_affinity), + &vcpuaff->cpumap_hard, + nr_cpu_ids); + if ( !ret ) + ret = vcpu_set_hard_affinity(v, new_affinity); + if ( ret ) + goto setvcpuaffinity_out; + + /* + * For hard affinity, what we return is the intersection of + * cpupool's online mask and the new hard affinity. + */ + cpumask_and(new_affinity, online, v->cpu_hard_affinity); + ret = cpumask_to_xenctl_bitmap(&vcpuaff->cpumap_hard, + new_affinity); + } + if ( vcpuaff->flags & XEN_VCPUAFFINITY_SOFT ) + { + ret = xenctl_bitmap_to_bitmap(cpumask_bits(new_affinity), + &vcpuaff->cpumap_soft, + nr_cpu_ids); + if ( !ret) + ret = vcpu_set_soft_affinity(v, new_affinity); + if ( ret ) + { + /* + * Since we're returning error, the caller expects nothing + * happened, so we rollback the changes to hard affinity + * (if any). + */ + if ( vcpuaff->flags & XEN_VCPUAFFINITY_HARD ) + vcpu_set_hard_affinity(v, old_affinity); + goto setvcpuaffinity_out; + } + + /* + * For soft affinity, we return the intersection between the + * new soft affinity, the cpupool's online map and the (new) + * hard affinity. + */ + cpumask_and(new_affinity, new_affinity, online); + cpumask_and(new_affinity, new_affinity, v->cpu_hard_affinity); + ret = cpumask_to_xenctl_bitmap(&vcpuaff->cpumap_soft, + new_affinity); + } + + setvcpuaffinity_out: + free_cpumask_var(new_affinity); + free_cpumask_var(old_affinity); } else { - ret = cpumask_to_xenctl_bitmap( - &op->u.vcpuaffinity.cpumap, v->cpu_hard_affinity); + if ( vcpuaff->flags & XEN_VCPUAFFINITY_HARD ) + ret = cpumask_to_xenctl_bitmap(&vcpuaff->cpumap_hard, + v->cpu_hard_affinity); + if ( vcpuaff->flags & XEN_VCPUAFFINITY_SOFT ) + ret = cpumask_to_xenctl_bitmap(&vcpuaff->cpumap_soft, + v->cpu_soft_affinity); } } break; diff --git a/xen/common/schedule.c b/xen/common/schedule.c index e57cd9159a..e9eb0bc259 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -650,22 +650,14 @@ int cpu_disable_scheduler(unsigned int cpu) return ret; } -int vcpu_set_affinity(struct vcpu *v, const cpumask_t *affinity) +static int vcpu_set_affinity( + struct vcpu *v, const cpumask_t *affinity, cpumask_t *which) { - cpumask_t online_affinity; - cpumask_t *online; spinlock_t *lock; - if ( v->domain->is_pinned ) - return -EINVAL; - online = VCPU2ONLINE(v); - cpumask_and(&online_affinity, affinity, online); - if ( cpumask_empty(&online_affinity) ) - return -EINVAL; - lock = vcpu_schedule_lock_irq(v); - cpumask_copy(v->cpu_hard_affinity, affinity); + cpumask_copy(which, affinity); /* Always ask the scheduler to re-evaluate placement * when changing the affinity */ @@ -684,6 +676,27 @@ int vcpu_set_affinity(struct vcpu *v, const cpumask_t *affinity) return 0; } +int vcpu_set_hard_affinity(struct vcpu *v, const cpumask_t *affinity) +{ + cpumask_t online_affinity; + cpumask_t *online; + + if ( v->domain->is_pinned ) + return -EINVAL; + + online = VCPU2ONLINE(v); + cpumask_and(&online_affinity, affinity, online); + if ( cpumask_empty(&online_affinity) ) + return -EINVAL; + + return vcpu_set_affinity(v, affinity, v->cpu_hard_affinity); +} + +int vcpu_set_soft_affinity(struct vcpu *v, const cpumask_t *affinity) +{ + return vcpu_set_affinity(v, affinity, v->cpu_soft_affinity); +} + /* Block the currently-executing domain until a pertinent event occurs. */ void vcpu_block(void) { diff --git a/xen/common/wait.c b/xen/common/wait.c index 3f6ff410e2..1f6b597657 100644 --- a/xen/common/wait.c +++ b/xen/common/wait.c @@ -135,7 +135,7 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv) /* Save current VCPU affinity; force wakeup on *this* CPU only. */ wqv->wakeup_cpu = smp_processor_id(); cpumask_copy(&wqv->saved_affinity, curr->cpu_hard_affinity); - if ( vcpu_set_affinity(curr, cpumask_of(wqv->wakeup_cpu)) ) + if ( vcpu_set_hard_affinity(curr, cpumask_of(wqv->wakeup_cpu)) ) { gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n"); domain_crash_synchronous(); @@ -166,7 +166,7 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv) static void __finish_wait(struct waitqueue_vcpu *wqv) { wqv->esp = NULL; - (void)vcpu_set_affinity(current, &wqv->saved_affinity); + (void)vcpu_set_hard_affinity(current, &wqv->saved_affinity); } void check_wakeup_from_wait(void) @@ -184,7 +184,7 @@ void check_wakeup_from_wait(void) /* Re-set VCPU affinity and re-enter the scheduler. */ struct vcpu *curr = current; cpumask_copy(&wqv->saved_affinity, curr->cpu_hard_affinity); - if ( vcpu_set_affinity(curr, cpumask_of(wqv->wakeup_cpu)) ) + if ( vcpu_set_hard_affinity(curr, cpumask_of(wqv->wakeup_cpu)) ) { gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n"); domain_crash_synchronous(); diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 2967133c91..5b11bbfac4 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -300,8 +300,33 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_nodeaffinity_t); /* XEN_DOMCTL_setvcpuaffinity */ /* XEN_DOMCTL_getvcpuaffinity */ struct xen_domctl_vcpuaffinity { - uint32_t vcpu; /* IN */ - struct xenctl_bitmap cpumap; /* IN/OUT */ + /* IN variables. */ + uint32_t vcpu; + /* Set/get the hard affinity for vcpu */ +#define _XEN_VCPUAFFINITY_HARD 0 +#define XEN_VCPUAFFINITY_HARD (1U<<_XEN_VCPUAFFINITY_HARD) + /* Set/get the soft affinity for vcpu */ +#define _XEN_VCPUAFFINITY_SOFT 1 +#define XEN_VCPUAFFINITY_SOFT (1U<<_XEN_VCPUAFFINITY_SOFT) + uint32_t flags; + /* + * IN/OUT variables. + * + * Both are IN/OUT for XEN_DOMCTL_setvcpuaffinity, in which case they + * contain effective hard or/and soft affinity. That is, upon successful + * return, cpumap_soft, contains the intersection of the soft affinity, + * hard affinity and the cpupool's online CPUs for the domain (if + * XEN_VCPUAFFINITY_SOFT was set in flags). cpumap_hard contains the + * intersection between hard affinity and the cpupool's online CPUs (if + * XEN_VCPUAFFINITY_HARD was set in flags). + * + * Both are OUT-only for XEN_DOMCTL_getvcpuaffinity, in which case they + * contain the plain hard and/or soft affinity masks that were set during + * previous successful calls to XEN_DOMCTL_setvcpuaffinity (or the + * default values), without intersecting or altering them in any way. + */ + struct xenctl_bitmap cpumap_hard; + struct xenctl_bitmap cpumap_soft; }; typedef struct xen_domctl_vcpuaffinity xen_domctl_vcpuaffinity_t; DEFINE_XEN_GUEST_HANDLE(xen_domctl_vcpuaffinity_t); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 445b659dd2..f920e1a3be 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -781,7 +781,8 @@ void scheduler_free(struct scheduler *sched); int schedule_cpu_switch(unsigned int cpu, struct cpupool *c); void vcpu_force_reschedule(struct vcpu *v); int cpu_disable_scheduler(unsigned int cpu); -int vcpu_set_affinity(struct vcpu *v, const cpumask_t *affinity); +int vcpu_set_hard_affinity(struct vcpu *v, const cpumask_t *affinity); +int vcpu_set_soft_affinity(struct vcpu *v, const cpumask_t *affinity); void restore_vcpu_affinity(struct domain *d); void vcpu_runstate_get(struct vcpu *v, struct vcpu_runstate_info *runstate);