This patch provides a caching mechanism for the device address
extensions uid and fid on S390. For efficient sparse address allocation,
we introduce two hash tables for uid/fid which hold the address set
information per domain. Also in order to improve performance of
searching available value, we introduce our own callbacks for the two
hashtables. In this way, uid/fid is saved in hash key and hash value
could be any non-NULL pointer due to no operation on hash value. That is
also the reason why we don't introduce hash value free callback.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
src/conf/domain_addr.c | 85 ++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_addr.h | 9 +++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_domain_address.c | 7 ++++
4 files changed, 102 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 16f7ffa928..8d3e75f19a 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -27,11 +27,23 @@
#include "virlog.h"
#include "virstring.h"
#include "domain_addr.h"
+#include "virhashcode.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
VIR_LOG_INIT("conf.domain_addr");
+static void
+virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
+{
+ if (!addrs || !addrs->zpciIds)
+ return;
+
+ virHashFree(addrs->zpciIds->uids);
+ virHashFree(addrs->zpciIds->fids);
+ VIR_FREE(addrs->zpciIds);
+}
+
virDomainPCIConnectFlags
virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model)
{
@@ -741,6 +753,78 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs,
addrs->buses[addr->bus].slot[addr->slot].functions &= ~(1 << addr->function);
}
+
+static uint32_t
+virZPCIAddrCode(const void *name,
+ uint32_t seed)
+{
+ unsigned int value = *((unsigned int *)name);
+ return virHashCodeGen(&value, sizeof(value), seed);
+}
+
+
+static bool
+virZPCIAddrEqual(const void *namea,
+ const void *nameb)
+{
+ return *((unsigned int *)namea) == *((unsigned int *)nameb);
+}
+
+
+static void *
+virZPCIAddrCopy(const void *name)
+{
+ unsigned int *copy;
+
+ if (VIR_ALLOC(copy) < 0)
+ return NULL;
+
+ *copy = *((unsigned int *)name);
+ return (void *)copy;
+}
+
+
+static void
+virZPCIAddrKeyFree(void *name)
+{
+ VIR_FREE(name);
+}
+
+
+int
+virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs,
+ virDomainPCIAddressExtensionFlags extFlags)
+{
+ if (extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) {
+ if (addrs->zpciIds)
+ return 0;
+
+ if (VIR_ALLOC(addrs->zpciIds) < 0)
+ return -1;
+
+ if (!(addrs->zpciIds->uids = virHashCreateFull(10, NULL,
+ virZPCIAddrCode,
+ virZPCIAddrEqual,
+ virZPCIAddrCopy,
+ virZPCIAddrKeyFree)))
+ goto error;
+
+ if (!(addrs->zpciIds->fids = virHashCreateFull(10, NULL,
+ virZPCIAddrCode,
+ virZPCIAddrEqual,
+ virZPCIAddrCopy,
+ virZPCIAddrKeyFree)))
+ goto error;
+ }
+
+ return 0;
+
+ error:
+ virDomainPCIAddressSetExtensionFree(addrs);
+ return -1;
+}
+
+
virDomainPCIAddressSetPtr
virDomainPCIAddressSetAlloc(unsigned int nbuses)
{
@@ -767,6 +851,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs)
if (!addrs)
return;
+ virDomainPCIAddressSetExtensionFree(addrs);
VIR_FREE(addrs->buses);
VIR_FREE(addrs);
}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 5219d2f208..d1ec869da4 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -116,6 +116,11 @@ typedef struct {
} virDomainPCIAddressBus;
typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr;
+typedef struct {
+ virHashTablePtr uids, fids;
+} virDomainZPCIAddressIds;
+typedef virDomainZPCIAddressIds *virDomainZPCIAddressIdsPtr;
+
struct _virDomainPCIAddressSet {
virDomainPCIAddressBus *buses;
size_t nbuses;
@@ -125,6 +130,7 @@ struct _virDomainPCIAddressSet {
bool areMultipleRootsSupported;
/* If true, the guest can use the pcie-to-pci-bridge controller */
bool isPCIeToPCIBridgeSupported;
+ virDomainZPCIAddressIdsPtr zpciIds;
};
typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet;
typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
@@ -132,6 +138,9 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
ATTRIBUTE_NONNULL(1);
+int virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs,
+ virDomainPCIAddressExtensionFlags extFlags);
+
virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses);
void virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 70dfcc5e29..d57a836c2e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -116,6 +116,7 @@ virDomainPCIAddressReserveAddr;
virDomainPCIAddressReserveNextAddr;
virDomainPCIAddressSetAllMulti;
virDomainPCIAddressSetAlloc;
+virDomainPCIAddressSetExtensionAlloc;
virDomainPCIAddressSetFree;
virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index c582a531db..200d96a790 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1482,6 +1482,13 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
addrs->dryRun = dryRun;
+ /* create zpci address set for s390 domain */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI) &&
+ virDomainPCIAddressSetExtensionAlloc(addrs,
+ VIR_PCI_ADDRESS_EXTENSION_ZPCI)) {
+ goto error;
+ }
+
/* pSeries domains support multiple pci-root controllers */
if (qemuDomainIsPSeries(def))
addrs->areMultipleRootsSupported = true;
--
Yi Min
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote: > This patch provides a caching mechanism for the device address > extensions uid and fid on S390. For efficient sparse address allocation, > we introduce two hash tables for uid/fid which hold the address set > information per domain. Also in order to improve performance of > searching available value, we introduce our own callbacks for the two > hashtables. In this way, uid/fid is saved in hash key and hash value > could be any non-NULL pointer due to no operation on hash value. That is > also the reason why we don't introduce hash value free callback. > > Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> > Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> > Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> > Reviewed-by: Ján Tomko <jtomko@redhat.com> > --- > src/conf/domain_addr.c | 85 ++++++++++++++++++++++++++++++++++++++++++ > src/conf/domain_addr.h | 9 +++++ > src/libvirt_private.syms | 1 + > src/qemu/qemu_domain_address.c | 7 ++++ > 4 files changed, 102 insertions(+) I haven't looked into the hash table handling in detail but I wonder if it's really necessary? IIUC you're using it just to mark which uids and fids have been already used by a device, which the PCI address allocation code does by setting bits inside integer variables - having a custom hash table for the same seems like overkill, and from the maintenance point of view it would be great to have the logic for PCI address and zPCI address allocation be similar unless diverging is strictly necessary. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
在 2018/8/16 下午11:03, Andrea Bolognani 写道: > On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote: >> This patch provides a caching mechanism for the device address >> extensions uid and fid on S390. For efficient sparse address allocation, >> we introduce two hash tables for uid/fid which hold the address set >> information per domain. Also in order to improve performance of >> searching available value, we introduce our own callbacks for the two >> hashtables. In this way, uid/fid is saved in hash key and hash value >> could be any non-NULL pointer due to no operation on hash value. That is >> also the reason why we don't introduce hash value free callback. >> >> Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> >> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> >> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> >> Reviewed-by: Ján Tomko <jtomko@redhat.com> >> --- >> src/conf/domain_addr.c | 85 ++++++++++++++++++++++++++++++++++++++++++ >> src/conf/domain_addr.h | 9 +++++ >> src/libvirt_private.syms | 1 + >> src/qemu/qemu_domain_address.c | 7 ++++ >> 4 files changed, 102 insertions(+) > I haven't looked into the hash table handling in detail but I > wonder if it's really necessary? IIUC you're using it just to > mark which uids and fids have been already used by a device, > which the PCI address allocation code does by setting bits > inside integer variables - having a custom hash table for the > same seems like overkill, and from the maintenance point of > view it would be great to have the logic for PCI address and > zPCI address allocation be similar unless diverging is strictly > necessary. > PCI address set uses array to store pci addresses' assignment. It doesn't need too much memory because the buses are allocated dynamically, one bus only has 32 slot, and it's a tree topology. But in zpci case, fid and uid are flat. FID is 32-bit so that we need a 4294967295 sized array. Don't you think it's too large? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, 2018-08-20 at 16:32 +0800, Yi Min Zhao wrote: > 在 2018/8/16 下午11:03, Andrea Bolognani 写道: > > I haven't looked into the hash table handling in detail but I > > wonder if it's really necessary? IIUC you're using it just to > > mark which uids and fids have been already used by a device, > > which the PCI address allocation code does by setting bits > > inside integer variables - having a custom hash table for the > > same seems like overkill, and from the maintenance point of > > view it would be great to have the logic for PCI address and > > zPCI address allocation be similar unless diverging is strictly > > necessary. > > PCI address set uses array to store pci addresses' assignment. It doesn't > need too much memory because the buses are allocated dynamically, one > bus only has 32 slot, and it's a tree topology. But in zpci case, fid > and uid > are flat. FID is 32-bit so that we need a 4294967295 sized array. > Don't you think it's too large? Welp, I guess you're right. Disregard this comment then. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
在 2018/8/20 下午6:48, Andrea Bolognani 写道: > On Mon, 2018-08-20 at 16:32 +0800, Yi Min Zhao wrote: >> 在 2018/8/16 下午11:03, Andrea Bolognani 写道: >>> I haven't looked into the hash table handling in detail but I >>> wonder if it's really necessary? IIUC you're using it just to >>> mark which uids and fids have been already used by a device, >>> which the PCI address allocation code does by setting bits >>> inside integer variables - having a custom hash table for the >>> same seems like overkill, and from the maintenance point of >>> view it would be great to have the logic for PCI address and >>> zPCI address allocation be similar unless diverging is strictly >>> necessary. >> PCI address set uses array to store pci addresses' assignment. It doesn't >> need too much memory because the buses are allocated dynamically, one >> bus only has 32 slot, and it's a tree topology. But in zpci case, fid >> and uid >> are flat. FID is 32-bit so that we need a 4294967295 sized array. >> Don't you think it's too large? > Welp, I guess you're right. Disregard this comment then. > Thanks for your comment anyway! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.