* [PATCH 4.14 065/159] x86/virt: Add enum for hypervisors to replace x86_hyper
From: Greg Kroah-Hartman @ 2017-12-22 8:45 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, rkrcmar, pv-drivers, akataria, virtualization,
Thomas Gleixner, sthemmin, moltmann, Ingo Molnar, Peter Zijlstra,
linux-graphics-maintainer, linux-input, xen-devel, arnd,
Xavier Deguillard, haiyangz, devel, boris.ostrovsky,
Juergen Gross, Greg Kroah-Hartman, dmitry.torokhov, stable,
pbonzini, Linus Torvalds
In-Reply-To: <20171222084623.668990192@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Juergen Gross <jgross@suse.com>
commit 03b2a320b19f1424e9ac9c21696be9c60b6d0d93 upstream.
The x86_hyper pointer is only used for checking whether a virtual
device is supporting the hypervisor the system is running on.
Use an enum for that purpose instead and drop the x86_hyper pointer.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Xavier Deguillard <xdeguillard@vmware.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: akataria@vmware.com
Cc: arnd@arndb.de
Cc: boris.ostrovsky@oracle.com
Cc: devel@linuxdriverproject.org
Cc: dmitry.torokhov@gmail.com
Cc: gregkh@linuxfoundation.org
Cc: haiyangz@microsoft.com
Cc: kvm@vger.kernel.org
Cc: kys@microsoft.com
Cc: linux-graphics-maintainer@vmware.com
Cc: linux-input@vger.kernel.org
Cc: moltmann@vmware.com
Cc: pbonzini@redhat.com
Cc: pv-drivers@vmware.com
Cc: rkrcmar@redhat.com
Cc: sthemmin@microsoft.com
Cc: virtualization@lists.linux-foundation.org
Cc: xen-devel@lists.xenproject.org
Link: http://lkml.kernel.org/r/20171109132739.23465-3-jgross@suse.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/x86/hyperv/hv_init.c | 2 +-
arch/x86/include/asm/hypervisor.h | 23 ++++++++++++++---------
arch/x86/kernel/cpu/hypervisor.c | 12 +++++++++---
arch/x86/kernel/cpu/mshyperv.c | 4 ++--
arch/x86/kernel/cpu/vmware.c | 4 ++--
arch/x86/kernel/kvm.c | 4 ++--
arch/x86/xen/enlighten_hvm.c | 4 ++--
arch/x86/xen/enlighten_pv.c | 4 ++--
drivers/hv/vmbus_drv.c | 2 +-
drivers/input/mouse/vmmouse.c | 10 ++++------
drivers/misc/vmw_balloon.c | 2 +-
11 files changed, 40 insertions(+), 31 deletions(-)
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -113,7 +113,7 @@ void hyperv_init(void)
u64 guest_id;
union hv_x64_msr_hypercall_contents hypercall_msr;
- if (x86_hyper != &x86_hyper_ms_hyperv)
+ if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return;
/* Allocate percpu VP index */
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -29,6 +29,16 @@
/*
* x86 hypervisor information
*/
+
+enum x86_hypervisor_type {
+ X86_HYPER_NATIVE = 0,
+ X86_HYPER_VMWARE,
+ X86_HYPER_MS_HYPERV,
+ X86_HYPER_XEN_PV,
+ X86_HYPER_XEN_HVM,
+ X86_HYPER_KVM,
+};
+
struct hypervisor_x86 {
/* Hypervisor name */
const char *name;
@@ -36,6 +46,9 @@ struct hypervisor_x86 {
/* Detection routine */
uint32_t (*detect)(void);
+ /* Hypervisor type */
+ enum x86_hypervisor_type type;
+
/* init time callbacks */
struct x86_hyper_init init;
@@ -43,15 +56,7 @@ struct hypervisor_x86 {
struct x86_hyper_runtime runtime;
};
-extern const struct hypervisor_x86 *x86_hyper;
-
-/* Recognized hypervisors */
-extern const struct hypervisor_x86 x86_hyper_vmware;
-extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
-extern const struct hypervisor_x86 x86_hyper_xen_pv;
-extern const struct hypervisor_x86 x86_hyper_xen_hvm;
-extern const struct hypervisor_x86 x86_hyper_kvm;
-
+extern enum x86_hypervisor_type x86_hyper_type;
extern void init_hypervisor_platform(void);
#else
static inline void init_hypervisor_platform(void) { }
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -26,6 +26,12 @@
#include <asm/processor.h>
#include <asm/hypervisor.h>
+extern const struct hypervisor_x86 x86_hyper_vmware;
+extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
+extern const struct hypervisor_x86 x86_hyper_xen_pv;
+extern const struct hypervisor_x86 x86_hyper_xen_hvm;
+extern const struct hypervisor_x86 x86_hyper_kvm;
+
static const __initconst struct hypervisor_x86 * const hypervisors[] =
{
#ifdef CONFIG_XEN_PV
@@ -41,8 +47,8 @@ static const __initconst struct hypervis
#endif
};
-const struct hypervisor_x86 *x86_hyper;
-EXPORT_SYMBOL(x86_hyper);
+enum x86_hypervisor_type x86_hyper_type;
+EXPORT_SYMBOL(x86_hyper_type);
static inline const struct hypervisor_x86 * __init
detect_hypervisor_vendor(void)
@@ -87,6 +93,6 @@ void __init init_hypervisor_platform(voi
copy_array(&h->init, &x86_init.hyper, sizeof(h->init));
copy_array(&h->runtime, &x86_platform.hyper, sizeof(h->runtime));
- x86_hyper = h;
+ x86_hyper_type = h->type;
x86_init.hyper.init_platform();
}
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -254,9 +254,9 @@ static void __init ms_hyperv_init_platfo
#endif
}
-const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
+const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
.name = "Microsoft Hyper-V",
.detect = ms_hyperv_platform,
+ .type = X86_HYPER_MS_HYPERV,
.init.init_platform = ms_hyperv_init_platform,
};
-EXPORT_SYMBOL(x86_hyper_ms_hyperv);
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -205,10 +205,10 @@ static bool __init vmware_legacy_x2apic_
(eax & (1 << VMWARE_PORT_CMD_LEGACY_X2APIC)) != 0;
}
-const __refconst struct hypervisor_x86 x86_hyper_vmware = {
+const __initconst struct hypervisor_x86 x86_hyper_vmware = {
.name = "VMware",
.detect = vmware_platform,
+ .type = X86_HYPER_VMWARE,
.init.init_platform = vmware_platform_setup,
.init.x2apic_available = vmware_legacy_x2apic_available,
};
-EXPORT_SYMBOL(x86_hyper_vmware);
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -544,12 +544,12 @@ static uint32_t __init kvm_detect(void)
return kvm_cpuid_base();
}
-const struct hypervisor_x86 x86_hyper_kvm __refconst = {
+const __initconst struct hypervisor_x86 x86_hyper_kvm = {
.name = "KVM",
.detect = kvm_detect,
+ .type = X86_HYPER_KVM,
.init.x2apic_available = kvm_para_available,
};
-EXPORT_SYMBOL_GPL(x86_hyper_kvm);
static __init int activate_jump_labels(void)
{
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -226,12 +226,12 @@ static uint32_t __init xen_platform_hvm(
return xen_cpuid_base();
}
-const struct hypervisor_x86 x86_hyper_xen_hvm = {
+const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
.name = "Xen HVM",
.detect = xen_platform_hvm,
+ .type = X86_HYPER_XEN_HVM,
.init.init_platform = xen_hvm_guest_init,
.init.x2apic_available = xen_x2apic_para_available,
.init.init_mem_mapping = xen_hvm_init_mem_mapping,
.runtime.pin_vcpu = xen_pin_vcpu,
};
-EXPORT_SYMBOL(x86_hyper_xen_hvm);
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1459,9 +1459,9 @@ static uint32_t __init xen_platform_pv(v
return 0;
}
-const struct hypervisor_x86 x86_hyper_xen_pv = {
+const __initconst struct hypervisor_x86 x86_hyper_xen_pv = {
.name = "Xen PV",
.detect = xen_platform_pv,
+ .type = X86_HYPER_XEN_PV,
.runtime.pin_vcpu = xen_pin_vcpu,
};
-EXPORT_SYMBOL(x86_hyper_xen_pv);
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1534,7 +1534,7 @@ static int __init hv_acpi_init(void)
{
int ret, t;
- if (x86_hyper != &x86_hyper_ms_hyperv)
+ if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return -ENODEV;
init_completion(&probe_event);
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -316,11 +316,9 @@ static int vmmouse_enable(struct psmouse
/*
* Array of supported hypervisors.
*/
-static const struct hypervisor_x86 *vmmouse_supported_hypervisors[] = {
- &x86_hyper_vmware,
-#ifdef CONFIG_KVM_GUEST
- &x86_hyper_kvm,
-#endif
+static enum x86_hypervisor_type vmmouse_supported_hypervisors[] = {
+ X86_HYPER_VMWARE,
+ X86_HYPER_KVM,
};
/**
@@ -331,7 +329,7 @@ static bool vmmouse_check_hypervisor(voi
int i;
for (i = 0; i < ARRAY_SIZE(vmmouse_supported_hypervisors); i++)
- if (vmmouse_supported_hypervisors[i] == x86_hyper)
+ if (vmmouse_supported_hypervisors[i] == x86_hyper_type)
return true;
return false;
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -1271,7 +1271,7 @@ static int __init vmballoon_init(void)
* Check if we are running on VMware's hypervisor and bail out
* if we are not.
*/
- if (x86_hyper != &x86_hyper_vmware)
+ if (x86_hyper_type != X86_HYPER_VMWARE)
return -ENODEV;
for (is_2m_pages = 0; is_2m_pages < VMW_BALLOON_NUM_PAGE_SIZES;
^ permalink raw reply
* [PATCH 4.14 064/159] x86/virt, x86/platform: Merge struct x86_hyper into struct x86_platform and struct x86_init
From: Greg Kroah-Hartman @ 2017-12-22 8:45 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, sthemmin, xen-devel, kvm, rkrcmar, Peter Zijlstra,
Greg Kroah-Hartman, boris.ostrovsky, rusty, akataria, stable,
virtualization, haiyangz, pbonzini, devel, Thomas Gleixner,
Linus Torvalds, Ingo Molnar
In-Reply-To: <20171222084623.668990192@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Juergen Gross <jgross@suse.com>
commit f72e38e8ec8869ac0ba5a75d7d2f897d98a1454e upstream.
Instead of x86_hyper being either NULL on bare metal or a pointer to a
struct hypervisor_x86 in case of the kernel running as a guest merge
the struct into x86_platform and x86_init.
This will remove the need for wrappers making it hard to find out what
is being called. With dummy functions added for all callbacks testing
for a NULL function pointer can be removed, too.
Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: akataria@vmware.com
Cc: boris.ostrovsky@oracle.com
Cc: devel@linuxdriverproject.org
Cc: haiyangz@microsoft.com
Cc: kvm@vger.kernel.org
Cc: kys@microsoft.com
Cc: pbonzini@redhat.com
Cc: rkrcmar@redhat.com
Cc: rusty@rustcorp.com.au
Cc: sthemmin@microsoft.com
Cc: virtualization@lists.linux-foundation.org
Cc: xen-devel@lists.xenproject.org
Link: http://lkml.kernel.org/r/20171109132739.23465-2-jgross@suse.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/x86/include/asm/hypervisor.h | 25 +++-------------
arch/x86/include/asm/x86_init.h | 24 ++++++++++++++++
arch/x86/kernel/apic/apic.c | 2 -
arch/x86/kernel/cpu/hypervisor.c | 56 ++++++++++++++++++--------------------
arch/x86/kernel/cpu/mshyperv.c | 2 -
arch/x86/kernel/cpu/vmware.c | 4 +-
arch/x86/kernel/kvm.c | 2 -
arch/x86/kernel/x86_init.c | 9 ++++++
arch/x86/mm/init.c | 2 -
arch/x86/xen/enlighten_hvm.c | 8 ++---
arch/x86/xen/enlighten_pv.c | 2 -
include/linux/hypervisor.h | 8 ++++-
12 files changed, 82 insertions(+), 62 deletions(-)
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -23,6 +23,7 @@
#ifdef CONFIG_HYPERVISOR_GUEST
#include <asm/kvm_para.h>
+#include <asm/x86_init.h>
#include <asm/xen/hypervisor.h>
/*
@@ -35,17 +36,11 @@ struct hypervisor_x86 {
/* Detection routine */
uint32_t (*detect)(void);
- /* Platform setup (run once per boot) */
- void (*init_platform)(void);
+ /* init time callbacks */
+ struct x86_hyper_init init;
- /* X2APIC detection (run once per boot) */
- bool (*x2apic_available)(void);
-
- /* pin current vcpu to specified physical cpu (run rarely) */
- void (*pin_vcpu)(int);
-
- /* called during init_mem_mapping() to setup early mappings. */
- void (*init_mem_mapping)(void);
+ /* runtime callbacks */
+ struct x86_hyper_runtime runtime;
};
extern const struct hypervisor_x86 *x86_hyper;
@@ -58,17 +53,7 @@ extern const struct hypervisor_x86 x86_h
extern const struct hypervisor_x86 x86_hyper_kvm;
extern void init_hypervisor_platform(void);
-extern bool hypervisor_x2apic_available(void);
-extern void hypervisor_pin_vcpu(int cpu);
-
-static inline void hypervisor_init_mem_mapping(void)
-{
- if (x86_hyper && x86_hyper->init_mem_mapping)
- x86_hyper->init_mem_mapping();
-}
#else
static inline void init_hypervisor_platform(void) { }
-static inline bool hypervisor_x2apic_available(void) { return false; }
-static inline void hypervisor_init_mem_mapping(void) { }
#endif /* CONFIG_HYPERVISOR_GUEST */
#endif /* _ASM_X86_HYPERVISOR_H */
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -115,6 +115,18 @@ struct x86_init_pci {
};
/**
+ * struct x86_hyper_init - x86 hypervisor init functions
+ * @init_platform: platform setup
+ * @x2apic_available: X2APIC detection
+ * @init_mem_mapping: setup early mappings during init_mem_mapping()
+ */
+struct x86_hyper_init {
+ void (*init_platform)(void);
+ bool (*x2apic_available)(void);
+ void (*init_mem_mapping)(void);
+};
+
+/**
* struct x86_init_ops - functions for platform specific setup
*
*/
@@ -127,6 +139,7 @@ struct x86_init_ops {
struct x86_init_timers timers;
struct x86_init_iommu iommu;
struct x86_init_pci pci;
+ struct x86_hyper_init hyper;
};
/**
@@ -200,6 +213,15 @@ struct x86_legacy_features {
};
/**
+ * struct x86_hyper_runtime - x86 hypervisor specific runtime callbacks
+ *
+ * @pin_vcpu: pin current vcpu to specified physical cpu (run rarely)
+ */
+struct x86_hyper_runtime {
+ void (*pin_vcpu)(int cpu);
+};
+
+/**
* struct x86_platform_ops - platform specific runtime functions
* @calibrate_cpu: calibrate CPU
* @calibrate_tsc: calibrate TSC, if different from CPU
@@ -218,6 +240,7 @@ struct x86_legacy_features {
* possible in x86_early_init_platform_quirks() by
* only using the current x86_hardware_subarch
* semantics.
+ * @hyper: x86 hypervisor specific runtime callbacks
*/
struct x86_platform_ops {
unsigned long (*calibrate_cpu)(void);
@@ -233,6 +256,7 @@ struct x86_platform_ops {
void (*apic_post_init)(void);
struct x86_legacy_features legacy;
void (*set_legacy_features)(void);
+ struct x86_hyper_runtime hyper;
};
struct pci_dev;
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1645,7 +1645,7 @@ static __init void try_to_enable_x2apic(
* under KVM
*/
if (max_physical_apicid > 255 ||
- !hypervisor_x2apic_available()) {
+ !x86_init.hyper.x2apic_available()) {
pr_info("x2apic: IRQ remapping doesn't support X2APIC mode\n");
x2apic_disable();
return;
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -44,51 +44,49 @@ static const __initconst struct hypervis
const struct hypervisor_x86 *x86_hyper;
EXPORT_SYMBOL(x86_hyper);
-static inline void __init
+static inline const struct hypervisor_x86 * __init
detect_hypervisor_vendor(void)
{
- const struct hypervisor_x86 *h, * const *p;
+ const struct hypervisor_x86 *h = NULL, * const *p;
uint32_t pri, max_pri = 0;
for (p = hypervisors; p < hypervisors + ARRAY_SIZE(hypervisors); p++) {
- h = *p;
- pri = h->detect();
- if (pri != 0 && pri > max_pri) {
+ pri = (*p)->detect();
+ if (pri > max_pri) {
max_pri = pri;
- x86_hyper = h;
+ h = *p;
}
}
- if (max_pri)
- pr_info("Hypervisor detected: %s\n", x86_hyper->name);
-}
-
-void __init init_hypervisor_platform(void)
-{
-
- detect_hypervisor_vendor();
+ if (h)
+ pr_info("Hypervisor detected: %s\n", h->name);
- if (!x86_hyper)
- return;
-
- if (x86_hyper->init_platform)
- x86_hyper->init_platform();
+ return h;
}
-bool __init hypervisor_x2apic_available(void)
+static void __init copy_array(const void *src, void *target, unsigned int size)
{
- return x86_hyper &&
- x86_hyper->x2apic_available &&
- x86_hyper->x2apic_available();
+ unsigned int i, n = size / sizeof(void *);
+ const void * const *from = (const void * const *)src;
+ const void **to = (const void **)target;
+
+ for (i = 0; i < n; i++)
+ if (from[i])
+ to[i] = from[i];
}
-void hypervisor_pin_vcpu(int cpu)
+void __init init_hypervisor_platform(void)
{
- if (!x86_hyper)
+ const struct hypervisor_x86 *h;
+
+ h = detect_hypervisor_vendor();
+
+ if (!h)
return;
- if (x86_hyper->pin_vcpu)
- x86_hyper->pin_vcpu(cpu);
- else
- WARN_ONCE(1, "vcpu pinning requested but not supported!\n");
+ copy_array(&h->init, &x86_init.hyper, sizeof(h->init));
+ copy_array(&h->runtime, &x86_platform.hyper, sizeof(h->runtime));
+
+ x86_hyper = h;
+ x86_init.hyper.init_platform();
}
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -257,6 +257,6 @@ static void __init ms_hyperv_init_platfo
const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
.name = "Microsoft Hyper-V",
.detect = ms_hyperv_platform,
- .init_platform = ms_hyperv_init_platform,
+ .init.init_platform = ms_hyperv_init_platform,
};
EXPORT_SYMBOL(x86_hyper_ms_hyperv);
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -208,7 +208,7 @@ static bool __init vmware_legacy_x2apic_
const __refconst struct hypervisor_x86 x86_hyper_vmware = {
.name = "VMware",
.detect = vmware_platform,
- .init_platform = vmware_platform_setup,
- .x2apic_available = vmware_legacy_x2apic_available,
+ .init.init_platform = vmware_platform_setup,
+ .init.x2apic_available = vmware_legacy_x2apic_available,
};
EXPORT_SYMBOL(x86_hyper_vmware);
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -547,7 +547,7 @@ static uint32_t __init kvm_detect(void)
const struct hypervisor_x86 x86_hyper_kvm __refconst = {
.name = "KVM",
.detect = kvm_detect,
- .x2apic_available = kvm_para_available,
+ .init.x2apic_available = kvm_para_available,
};
EXPORT_SYMBOL_GPL(x86_hyper_kvm);
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -28,6 +28,8 @@ void x86_init_noop(void) { }
void __init x86_init_uint_noop(unsigned int unused) { }
int __init iommu_init_noop(void) { return 0; }
void iommu_shutdown_noop(void) { }
+bool __init bool_x86_init_noop(void) { return false; }
+void x86_op_int_noop(int cpu) { }
/*
* The platform setup functions are preset with the default functions
@@ -81,6 +83,12 @@ struct x86_init_ops x86_init __initdata
.init_irq = x86_default_pci_init_irq,
.fixup_irqs = x86_default_pci_fixup_irqs,
},
+
+ .hyper = {
+ .init_platform = x86_init_noop,
+ .x2apic_available = bool_x86_init_noop,
+ .init_mem_mapping = x86_init_noop,
+ },
};
struct x86_cpuinit_ops x86_cpuinit = {
@@ -101,6 +109,7 @@ struct x86_platform_ops x86_platform __r
.get_nmi_reason = default_get_nmi_reason,
.save_sched_clock_state = tsc_save_sched_clock_state,
.restore_sched_clock_state = tsc_restore_sched_clock_state,
+ .hyper.pin_vcpu = x86_op_int_noop,
};
EXPORT_SYMBOL_GPL(x86_platform);
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -671,7 +671,7 @@ void __init init_mem_mapping(void)
load_cr3(swapper_pg_dir);
__flush_tlb_all();
- hypervisor_init_mem_mapping();
+ x86_init.hyper.init_mem_mapping();
early_memtest(0, max_pfn_mapped << PAGE_SHIFT);
}
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -229,9 +229,9 @@ static uint32_t __init xen_platform_hvm(
const struct hypervisor_x86 x86_hyper_xen_hvm = {
.name = "Xen HVM",
.detect = xen_platform_hvm,
- .init_platform = xen_hvm_guest_init,
- .pin_vcpu = xen_pin_vcpu,
- .x2apic_available = xen_x2apic_para_available,
- .init_mem_mapping = xen_hvm_init_mem_mapping,
+ .init.init_platform = xen_hvm_guest_init,
+ .init.x2apic_available = xen_x2apic_para_available,
+ .init.init_mem_mapping = xen_hvm_init_mem_mapping,
+ .runtime.pin_vcpu = xen_pin_vcpu,
};
EXPORT_SYMBOL(x86_hyper_xen_hvm);
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1462,6 +1462,6 @@ static uint32_t __init xen_platform_pv(v
const struct hypervisor_x86 x86_hyper_xen_pv = {
.name = "Xen PV",
.detect = xen_platform_pv,
- .pin_vcpu = xen_pin_vcpu,
+ .runtime.pin_vcpu = xen_pin_vcpu,
};
EXPORT_SYMBOL(x86_hyper_xen_pv);
--- a/include/linux/hypervisor.h
+++ b/include/linux/hypervisor.h
@@ -7,8 +7,12 @@
* Juergen Gross <jgross@suse.com>
*/
-#ifdef CONFIG_HYPERVISOR_GUEST
-#include <asm/hypervisor.h>
+#ifdef CONFIG_X86
+#include <asm/x86_init.h>
+static inline void hypervisor_pin_vcpu(int cpu)
+{
+ x86_platform.hyper.pin_vcpu(cpu);
+}
#else
static inline void hypervisor_pin_vcpu(int cpu)
{
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Wei Wang @ 2017-12-22 8:45 UTC (permalink / raw)
To: Tetsuo Handa, willy
Cc: virtio-dev, kvm, mst, qemu-devel, mawilcox, linux-kernel,
virtualization, linux-mm, akpm, mhocko
In-Reply-To: <201712212337.JFC57368.tLFOJFVSFHMOOQ@I-love.SAKURA.ne.jp>
On 12/21/2017 10:37 PM, Tetsuo Handa wrote:
> Matthew Wilcox wrote:
>>> +/**
>>> + * xb_find_set - find the next set bit in a range of bits
>>> + * @xb: the xbitmap to search from
>>> + * @offset: the offset in the range to start searching
>>> + * @size: the size of the range
>>> + *
>>> + * Returns: the found bit or, @size if no set bit is found.
>>> + */
>>> +unsigned long xb_find_set(struct xb *xb, unsigned long size,
>>> + unsigned long offset)
>>> +{
>>> + struct radix_tree_root *root = &xb->xbrt;
>>> + struct radix_tree_node *node;
>>> + void __rcu **slot;
>>> + struct ida_bitmap *bitmap;
>>> + unsigned long index = offset / IDA_BITMAP_BITS;
>>> + unsigned long index_end = size / IDA_BITMAP_BITS;
>>> + unsigned long bit = offset % IDA_BITMAP_BITS;
>>> +
>>> + if (unlikely(offset >= size))
>>> + return size;
>>> +
>>> + while (index <= index_end) {
>>> + unsigned long ret;
>>> + unsigned int nbits = size - index * IDA_BITMAP_BITS;
>>> +
>>> + bitmap = __radix_tree_lookup(root, index, &node, &slot);
>>> +
>>> + if (!node && !bitmap)
>>> + return size;
>>> +
>>> + if (bitmap) {
>>> + if (nbits > IDA_BITMAP_BITS)
>>> + nbits = IDA_BITMAP_BITS;
>>> +
>>> + ret = find_next_bit(bitmap->bitmap, nbits, bit);
>>> + if (ret != nbits)
>>> + return ret + index * IDA_BITMAP_BITS;
>>> + }
>>> + bit = 0;
>>> + index++;
>>> + }
>>> +
>>> + return size;
>>> +}
>>> +EXPORT_SYMBOL(xb_find_set);
>> This is going to be slower than the implementation I sent yesterday. If I
>> call:
>> xb_init(xb);
>> xb_set_bit(xb, ULONG_MAX);
>> xb_find_set(xb, ULONG_MAX, 0);
>>
>> it's going to call __radix_tree_lookup() 16 quadrillion times.
>> My implementation will walk the tree precisely once.
>>
> Yes. Wei's patch still can not work.
> We should start reviewing Matthew's implementation.
It runs without any issue on my machine. I didn't generate an "xbitmap"
executable (I just found adding xbitmap executable causes a build error
due to a Makefile error), instead, I tested it within "main" and it
passed all the tests.
Matthew has implemented a new version, let's start from there.
Best,
Wei
^ permalink raw reply
* Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available
From: Siwei Liu @ 2017-12-22 8:42 UTC (permalink / raw)
To: Jakub Kicinski
Cc: mst, sridhar.samudrala, Alexander Duyck, virtualization, netdev,
David Miller
In-Reply-To: <20171220205204.4853f1d1@cakuba.netronome.com>
On Wed, Dec 20, 2017 at 8:52 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Wed, 20 Dec 2017 18:16:30 -0800, Siwei Liu wrote:
>> > The plan is to remove the delay and do the naming in the kernel.
>> > This was suggested by Lennart since udev is only doing naming policy
>> > because kernel names were not repeatable.
>> >
>> > This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.
>> >
>> > Patch is pending.
>>
>> While it's good to show VF with specific naming to indicate
>> enslavement, I wonder wouldn't it be better to hide this netdev at all
>> from the user space? IMHO this extra device is useless when being
>> enslaved and we may delegate controls (e.g. ethtool) over to the
>> para-virtual device instead? That way it's possible to eliminate the
>> possibility of additional udev setup or modification?
>>
>> I'm not sure if this is consistent with Windows guest or not, but I
>> don't find it _Linux_ user friendly that ethtool doesn't work on the
>> composite interface any more, and I have to end up with finding out
>> the correct enslaved VF I must operate on.
>
> Hiding "low level" netdevs comes up from time to time, and is more
> widely applicable than just to VF bonds. We should find a generic
> solution to that problem.
Wholeheartedly agreed.
Be it a generic virtual bond or virtio-net specific one, there should
be some common code between netvsc and virtio for this type of work.
For avoiding duplicated bugs, consistent (Linux) user experience,
future code refactoring/management, and whatever...
One thing worth to note is that, unlike the Hyper-V netvsc backend
there's currently no equivalent (fine-grained) Linux ndo_* driver
interface that is able to move around MAC address/VLAN filters at
run-time specifically. The OID_RECEIVE_FILTER_MOVE_FILTER request I
mean. That translates to one substantial difference in VF
plumbing/unplumbing sequence: you cannot move the MAC address around
to paravirt device until VF is fully unplugged out of the guest OS. I
don't know what backend changes to be proposed for virtio-net as
helper, but the common code needs to work with both flavors of data
path switching backends and do its job correctly.
Regards,
-Siwei
^ permalink raw reply
* Re: [PATCH RFC 1/4] crypto: engine - Permit to enqueue all async requests
From: Corentin Labbe @ 2017-12-22 8:41 UTC (permalink / raw)
To: Herbert Xu
Cc: alexandre.torgue, mst, linux-kernel, fabien.dessenne,
virtualization, linux-crypto, mcoquelin.stm32, davem,
linux-arm-kernel
In-Reply-To: <20171222065724.GA27149@gondor.apana.org.au>
On Fri, Dec 22, 2017 at 05:57:24PM +1100, Herbert Xu wrote:
> On Wed, Nov 29, 2017 at 09:41:18AM +0100, Corentin Labbe wrote:
> > The crypto engine could actually only enqueue hash and ablkcipher request.
> > This patch permit it to enqueue any type of crypto_async_request.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>
> This is going the wrong way. We do not want to expose any of the
> base types such as crypto_alg, crypto_async_request to end-users
> and that includes drivers. Only core API code should touch these
> base types.
>
Hello
It's you that was suggesting using crypto_async_request:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1474434.html
"The only wart with this scheme is that the drivers end up seeing
struct crypto_async_request and will need to convert that to the
respective request types but I couldn't really find a better way."
So I wait for any suggestion.
Regards
Corentin Labbe
^ permalink raw reply
* Re: [PATCH RFC 1/4] crypto: engine - Permit to enqueue all async requests
From: Herbert Xu @ 2017-12-22 6:57 UTC (permalink / raw)
To: Corentin Labbe
Cc: alexandre.torgue, mst, linux-kernel, fabien.dessenne,
virtualization, linux-crypto, mcoquelin.stm32, davem,
linux-arm-kernel
In-Reply-To: <20171129084121.9385-2-clabbe.montjoie@gmail.com>
On Wed, Nov 29, 2017 at 09:41:18AM +0100, Corentin Labbe wrote:
> The crypto engine could actually only enqueue hash and ablkcipher request.
> This patch permit it to enqueue any type of crypto_async_request.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
This is going the wrong way. We do not want to expose any of the
base types such as crypto_alg, crypto_async_request to end-users
and that includes drivers. Only core API code should touch these
base types.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] virtio: make VIRTIO a menuconfig to ease disabling it all
From: Michael Ellerman @ 2017-12-22 2:12 UTC (permalink / raw)
To: Vincent Legoll; +Cc: virtualization, Linux Kernel ML, mst
In-Reply-To: <CAEwRq=pVCUPGq4=JLOHQ4gQE0FC+Fotci8+DR_VupgB1BJJB1w@mail.gmail.com>
Vincent Legoll <vincent.legoll@gmail.com> writes:
> Hello,
>
> thanks for the help, and sorry for the poor patch,
>
> On Thu, Dec 21, 2017 at 11:53 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> This breaks all existing .configs *and* defconfigs that use VIRTIO.
>>
>> Please don't do that.
>>
>> If you make it default y then everything will continue to work.
>
> Do you want me to respin it with the added default ?
Yes please, either that or revert it.
cheers
^ permalink raw reply
* Call for papers - MICRADS 2018 - Salinas, Ecuador
From: MarLe @ 2017-12-22 0:02 UTC (permalink / raw)
To: virtualization
* Proceedings by Springer and indexed by Scopus, etc.
---------------------------------------------------------------------------------------------------------------------------
MICRADS´18 - The 2018 Multidisciplinary International Conference of Research Applied to Defense and Security
Salinas, Ecuador, 18 - 20 April 2018
http://www.micrads.org/
------------------------------------------------------------------------------------------------------------------------------------
SCOPE
MICRADS´18 - The 2018 Multidisciplinary International Conference of Research Applied to Defense and Security, to be held at Salinas, Ecuador, 18 - 20 April 2018, is an international forum for researchers and practitioners to present and discuss the most recent innovations, trends, results, experiences and concerns in the several perspectives of Defense and Security.
We are pleased to invite you to submit your papers to MICRADS´18. They can be written in English, Spanish or Portuguese. All submissions will be reviewed on the basis of relevance, originality, importance and clarity.
TOPICS
Submitted papers should be related with one or more of the main themes proposed for the Conference:
A) Information and Communication Technology in Education;
B) Computer vision in military applications;
C) Engineering Analysis and Signal Processing;
D) Cybersecurity and Cyberdefense;
E) Maritime Security and Safety;
F) Strategy, geopolitics and Oceanopolitics;
G) Defense planning;
H) Leadership (e-leadership);
I) Defense Economics;
J) Defense Logistics;
K) Health informatics in military applications;
L) Simulation in Military Application;
M) Computer Networks, Mobility and Pervasive Systems;
N) Military Marketing;
O) Military Physical Training;
P) Assistive Devices and Wearable Technology;
Q) Naval and Military Engineering;
R) Weapons and Combat Systems;
S) Operational Oceanography.
SUBMISSION AND DECISION
Submitted papers (until 6-page limit) must comply with the format standard (http://www.micrads.org/images/Templates.rar), be written in English, Spanish or Portuguese, must not have been published before, not be under review for any other conference or publication and not include any information leading to the authors? identification. Therefore, the authors? names, affiliations and bibliographic references should not be included in the version for evaluation by the Scientific Committee. This information should only be included in the camera-ready version, saved in Word format and uploaded at the conference management system.
All papers will be subjected to a ?double-blind review? by at least two members of the Scientific Committee.
Based on Scientific Committee evaluation, a paper can be rejected or accepted by the Conference Chairs. In the later case, it can be accepted as paper or poster.
The authors of papers accepted as posters must build and print a poster to be exhibited during the Conference. This poster must follow an A1 or A2 vertical format. The Conference can includes Work Sessions where these posters are presented and orally discussed, with a 7 minute limit per poster.
The authors of accepted papers will have 15 minutes to present their work in a Conference Work Session; approximately 5 minutes of discussion will follow each presentation.
PUBLICATION AND INDEXING
To ensure that an accepted paper is published, at least one of the authors must be fully registered by the 3 of February 2018, and the paper must comply with the suggested layout and page-limit (until 10 pages). Additionally, all recommended changes must be addressed by the authors before they submit the camera-ready version.
No more than one paper per registration will be published. An extra fee must be paid for publication of additional papers, with a maximum of one additional paper per registration. One registration permits only the participation of one author in the conference.
Papers can be written in English, Spanish or Portuguese. Accepted and registered papers will be published in Proceedings by Springer, in a book of its SIST series, and will be submitted for indexing by ISI, SCOPUS, EI-Compendex, SpingerLink, and Google Scholar.
IMPORTANT DATES
Paper Submission: January 14, 2018
Notification of Acceptance: February 14, 2018
Payment of Registration, to ensure the inclusion of an accepted paper in the conference proceedings: February 18, 2018.
Camera-ready Submission: February 21, 2018
-
Website o MICRADS'18: http://www.micrads.org/
-------
---
This email has been checked for viruses by AVG.
http://www.avg.com
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Matthew Wilcox @ 2017-12-21 21:03 UTC (permalink / raw)
To: Wei Wang
Cc: virtio-dev, kvm, mst, qemu-devel, penguin-kernel, mawilcox,
linux-kernel, virtualization, linux-mm, akpm, mhocko
In-Reply-To: <1513823406-43632-1-git-send-email-wei.w.wang@intel.com>
OK, here's a rewrite of xbitmap.
Compared to the version you sent:
- xb_find_set() is the rewrite I sent out yesterday.
- xb_find_clear() is a new implementation. I use the IDR_FREE tag to find
clear bits. This led to me finding a bug in radix_tree_for_each_tagged().
- xb_zero() is also a new implementation (replacing xb_clear_bit_range).
It should also be more efficient in deep trees.
- Did not accept your change to xb_set_bit(); I think it's important to
leave the radix tree node in place, so we're guaranteed to make progress
in low-memory situations. We're expecting the caller to retry, so the
memory should not leak.
- Redid a lot of the kernel-doc. Thanks for adding it! I removed mentions
of implementation details, leaving only information useful to those who
are using it.
Compared to the version I put out back in February:
- Accepted your change to xb_preload(); I think it's an improvement.
- Rewrote xb_set_bit() to use the radix_tree_iter_ family of functions.
Should improve performance for deep trees.
- Rewrote xb_clear_bit() for the same reason.
- Left out the use of radix tree exceptional entries. Thanks for taking
them out! Keeps it clearer for now; if they prove useful, we can put
them back in.
- Removed the use of __radix_tree_delete and __radix_tree_create, so drop
the changes to those functions.
Other miscellaneous notes
- I left xb_fill() in the header file, even though there's no implementation
yet. Wouldn't be hard to add once we have a user.
- Used SPDX tags instead of a license notice.
I think we need more test cases ... in reviewing this to send out,
I found a bug in xb_zero(), which I've only fixed because I don't have
time to write a test case for it.
diff --git a/include/linux/xbitmap.h b/include/linux/xbitmap.h
new file mode 100644
index 000000000000..c008309a9494
--- /dev/null
+++ b/include/linux/xbitmap.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * eXtensible Bitmaps
+ * Copyright (c) 2017 Microsoft Corporation
+ * Author: Matthew Wilcox <mawilcox@microsoft.com>
+ *
+ * eXtensible Bitmaps provide an unlimited-size sparse bitmap facility.
+ * All bits are initially zero.
+ *
+ * Locking is to be provided by the user. No xb_ function is safe to
+ * call concurrently with any other xb_ function.
+ */
+
+#include <linux/idr.h>
+
+struct xb {
+ struct radix_tree_root xbrt;
+};
+
+#define XB_INIT { \
+ .xbrt = RADIX_TREE_INIT(IDR_RT_MARKER | GFP_NOWAIT), \
+}
+#define DEFINE_XB(name) struct xb name = XB_INIT
+
+static inline void xb_init(struct xb *xb)
+{
+ INIT_RADIX_TREE(&xb->xbrt, IDR_RT_MARKER | GFP_NOWAIT);
+}
+
+int xb_set_bit(struct xb *xb, unsigned long bit);
+bool xb_test_bit(const struct xb *xb, unsigned long bit);
+void xb_clear_bit(struct xb *xb, unsigned long bit);
+void xb_zero(struct xb *xb, unsigned long min, unsigned long max);
+void xb_fill(struct xb *xb, unsigned long min, unsigned long max);
+bool xb_find_set(const struct xb *xb, unsigned long max, unsigned long *bit);
+bool xb_find_zero(const struct xb *xb, unsigned long max, unsigned long *bit);
+
+static inline bool xb_empty(const struct xb *xb)
+{
+ return radix_tree_empty(&xb->xbrt);
+}
+
+int __must_check xb_preload(gfp_t);
+
+static inline void xb_preload_end(void)
+{
+ preempt_enable();
+}
diff --git a/lib/Makefile b/lib/Makefile
index d11c48ec8ffd..08a8183c390b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -19,7 +19,7 @@ KCOV_INSTRUMENT_dynamic_debug.o := n
lib-y := ctype.o string.o vsprintf.o cmdline.o \
rbtree.o radix-tree.o dump_stack.o timerqueue.o\
- idr.o int_sqrt.o extable.o \
+ idr.o xbitmap.o int_sqrt.o extable.o \
sha1.o chacha20.o irq_regs.o argv_split.o \
flex_proportions.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o kobject_uevent.o \
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index c8d55565fafa..d2bd8feb7b85 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -37,7 +37,7 @@
#include <linux/rcupdate.h>
#include <linux/slab.h>
#include <linux/string.h>
-
+#include <linux/xbitmap.h>
/* Number of nodes in fully populated tree of given height */
static unsigned long height_to_maxnodes[RADIX_TREE_MAX_PATH + 1] __read_mostly;
@@ -77,6 +77,11 @@ static struct kmem_cache *radix_tree_node_cachep;
RADIX_TREE_MAP_SHIFT))
#define IDA_PRELOAD_SIZE (IDA_MAX_PATH * 2 - 1)
+#define XB_INDEX_BITS (BITS_PER_LONG - ilog2(IDA_BITMAP_BITS))
+#define XB_MAX_PATH (DIV_ROUND_UP(XB_INDEX_BITS, \
+ RADIX_TREE_MAP_SHIFT))
+#define XB_PRELOAD_SIZE (XB_MAX_PATH * 2 - 1)
+
/*
* Per-cpu pool of preloaded nodes
*/
@@ -1781,7 +1786,7 @@ void __rcu **radix_tree_next_chunk(const struct radix_tree_root *root,
child = rcu_dereference_raw(node->slots[offset]);
}
- if (!child)
+ if (!child && !is_idr(root))
goto restart;
if (child == RADIX_TREE_RETRY)
break;
@@ -2135,6 +2140,35 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
}
EXPORT_SYMBOL(ida_pre_get);
+/**
+ * xb_preload - preload for xb_set_bit()
+ * @gfp_mask: allocation mask to use for preloading
+ *
+ * Preallocate memory to use for the next call to xb_set_bit(). On success,
+ * return zero, with preemption disabled. On error, return -ENOMEM with
+ * preemption not disabled.
+ */
+int xb_preload(gfp_t gfp)
+{
+ if (!this_cpu_read(ida_bitmap)) {
+ struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp);
+
+ if (!bitmap)
+ return -ENOMEM;
+ /*
+ * The per-CPU variable is updated with preemption enabled.
+ * If the calling task is unlucky to be scheduled to another
+ * CPU which has no ida_bitmap allocation, it will be detected
+ * when setting a bit (i.e. xb_set_bit()).
+ */
+ bitmap = this_cpu_cmpxchg(ida_bitmap, NULL, bitmap);
+ kfree(bitmap);
+ }
+
+ return __radix_tree_preload(gfp, XB_PRELOAD_SIZE);
+}
+EXPORT_SYMBOL(xb_preload);
+
void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max)
diff --git a/lib/xbitmap.c b/lib/xbitmap.c
new file mode 100644
index 000000000000..d596ba247b45
--- /dev/null
+++ b/lib/xbitmap.c
@@ -0,0 +1,396 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * XBitmap implementation
+ * Copyright (c) 2017 Microsoft Corporation
+ * Author: Matthew Wilcox <mawilcox@microsoft.com>
+ */
+
+#include <linux/bitmap.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <linux/xbitmap.h>
+
+/**
+ * xb_set_bit() - Set a bit in the XBitmap.
+ * @xb: The XBitmap.
+ * @bit: Index of the bit to set.
+ *
+ * This function is used to set a bit in the xbitmap.
+ *
+ * Return: 0 on success. -ENOMEM if memory could not be allocated.
+ */
+int xb_set_bit(struct xb *xb, unsigned long bit)
+{
+ unsigned long index = bit / IDA_BITMAP_BITS;
+ struct radix_tree_root *root = &xb->xbrt;
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ struct ida_bitmap *bitmap;
+
+ bit %= IDA_BITMAP_BITS;
+ radix_tree_iter_init(&iter, index);
+ slot = idr_get_free_cmn(root, &iter, GFP_NOWAIT | __GFP_NOWARN, index);
+ if (IS_ERR(slot)) {
+ if (slot == ERR_PTR(-ENOSPC))
+ return 0; /* Already set */
+ return -ENOMEM;
+ }
+ bitmap = rcu_dereference_raw(*slot);
+ if (!bitmap) {
+ bitmap = this_cpu_xchg(ida_bitmap, NULL);
+ if (!bitmap)
+ return -ENOMEM;
+ memset(bitmap, 0, sizeof(*bitmap));
+ radix_tree_iter_replace(root, &iter, slot, bitmap);
+ }
+
+ __set_bit(bit, bitmap->bitmap);
+ if (bitmap_full(bitmap->bitmap, IDA_BITMAP_BITS))
+ radix_tree_iter_tag_clear(root, &iter, IDR_FREE);
+ return 0;
+}
+EXPORT_SYMBOL(xb_set_bit);
+
+/**
+ * xb_clear_bit() - Clear a bit in the XBitmap.
+ * @xb: The XBitmap.
+ * @bit: Index of the bit to clear.
+ *
+ * This function is used to clear a bit in the xbitmap.
+ */
+void xb_clear_bit(struct xb *xb, unsigned long bit)
+{
+ unsigned long index = bit / IDA_BITMAP_BITS;
+ struct radix_tree_root *root = &xb->xbrt;
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ struct ida_bitmap *bitmap;
+
+ bit %= IDA_BITMAP_BITS;
+ slot = radix_tree_iter_lookup(root, &iter, index);
+ if (!slot)
+ return;
+ bitmap = radix_tree_deref_slot(slot);
+ if (!bitmap)
+ return;
+
+ radix_tree_iter_tag_set(root, &iter, IDR_FREE);
+ __clear_bit(bit, bitmap->bitmap);
+ if (bitmap_empty(bitmap->bitmap, IDA_BITMAP_BITS)) {
+ kfree(bitmap);
+ radix_tree_iter_delete(root, &iter, slot);
+ }
+}
+EXPORT_SYMBOL(xb_clear_bit);
+
+/**
+ * xb_zero() - Clear a range of bits in the XBitmap.
+ * @xb: The XBitmap.
+ * @min: The first bit to clear.
+ * @max: The last bit to clear.
+ *
+ * This function is used to clear a range of bits in the xbitmap.
+ */
+void xb_zero(struct xb *xb, unsigned long min, unsigned long max)
+{
+ struct radix_tree_root *root = &xb->xbrt;
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ struct ida_bitmap *bitmap;
+ unsigned long index = min / IDA_BITMAP_BITS;
+ unsigned long first = min % IDA_BITMAP_BITS;
+ unsigned long maxindex = max / IDA_BITMAP_BITS;
+
+ radix_tree_for_each_slot(slot, root, &iter, index) {
+ unsigned long nbits = IDA_BITMAP_BITS;
+ if (index > maxindex)
+ break;
+ bitmap = radix_tree_deref_slot(slot);
+ if (!bitmap)
+ continue;
+ radix_tree_iter_tag_set(root, &iter, IDR_FREE);
+
+ if (!first && iter.index < maxindex)
+ goto delete;
+ if (iter.index == maxindex)
+ nbits = max % IDA_BITMAP_BITS + 1;
+ bitmap_clear(bitmap->bitmap, first, nbits);
+ first = 0;
+ if (bitmap_empty(bitmap->bitmap, IDA_BITMAP_BITS))
+ goto delete;
+ continue;
+delete:
+ kfree(bitmap);
+ radix_tree_iter_delete(root, &iter, slot);
+ }
+}
+EXPORT_SYMBOL(xb_zero);
+
+/**
+ * xb_test_bit() - Test a bit in the xbitmap.
+ * @xb: The XBitmap.
+ * @bit: Index of the bit to test.
+ *
+ * This function is used to test a bit in the xbitmap.
+ *
+ * Return: %true if the bit is set.
+ */
+bool xb_test_bit(const struct xb *xb, unsigned long bit)
+{
+ unsigned long index = bit / IDA_BITMAP_BITS;
+ struct ida_bitmap *bitmap = radix_tree_lookup(&xb->xbrt, index);
+
+ bit %= IDA_BITMAP_BITS;
+
+ if (!bitmap)
+ return false;
+ return test_bit(bit, bitmap->bitmap);
+}
+EXPORT_SYMBOL(xb_test_bit);
+
+/**
+ * xb_find_set() - Find the next set bit in a range of bits.
+ * @xb: The XBitmap.
+ * @max: The maximum position to search.
+ * @bit: The first bit to examine, and on exit, the found bit.
+ *
+ * On entry, @bit points to the index of the first bit to search. On exit,
+ * if this function returns %true, @bit will be updated to the index of the
+ * first found bit. It will not be updated if this function returns %false.
+ *
+ * Return: %true if a set bit was found.
+ */
+bool xb_find_set(const struct xb *xb, unsigned long max, unsigned long *bit)
+{
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ struct ida_bitmap *bitmap;
+ unsigned long index = *bit / IDA_BITMAP_BITS;
+ unsigned int first = *bit % IDA_BITMAP_BITS;
+ unsigned long maxindex = max / IDA_BITMAP_BITS;
+
+ radix_tree_for_each_slot(slot, &xb->xbrt, &iter, index) {
+ if (iter.index > maxindex)
+ break;
+ bitmap = radix_tree_deref_slot(slot);
+ if (bitmap) {
+ unsigned int nbits = IDA_BITMAP_BITS;
+ if (iter.index == maxindex)
+ nbits = max % IDA_BITMAP_BITS + 1;
+ first = find_next_bit(bitmap->bitmap, nbits, first);
+ if (first != nbits) {
+ *bit = first + iter.index * IDA_BITMAP_BITS;
+ return true;
+ }
+ }
+ first = 0;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(xb_find_set);
+
+/**
+ * xb_find_zero() - Find the next zero bit in a range of bits
+ * @xb: The XBitmap.
+ * @max: The maximum index to search.
+ * @bit: Pointer to an index.
+ *
+ * On entry, @bit points to the index of the first bit to search. On exit,
+ * if this function returns %true, @bit will be updated to the index of the
+ * first found bit. It will not be updated if this function returns %false.
+ *
+ * Return: %true if a clear bit was found.
+ */
+bool xb_find_zero(const struct xb *xb, unsigned long max, unsigned long *bit)
+{
+ void __rcu **slot;
+ struct radix_tree_iter iter;
+ struct ida_bitmap *bitmap;
+ unsigned long index = *bit / IDA_BITMAP_BITS;
+ unsigned long first = *bit % IDA_BITMAP_BITS;
+ unsigned long maxindex = max / IDA_BITMAP_BITS;
+
+ radix_tree_for_each_tagged(slot, &xb->xbrt, &iter, index, IDR_FREE) {
+ unsigned int nbits = IDA_BITMAP_BITS;
+ if (iter.index > maxindex)
+ return false;
+ bitmap = radix_tree_deref_slot(slot);
+ if (!bitmap)
+ break;
+ if (iter.index == maxindex)
+ nbits = max % IDA_BITMAP_BITS + 1;
+ first = find_next_zero_bit(bitmap->bitmap, nbits, first);
+ if (first != nbits)
+ break;
+ first = 0;
+ }
+
+ *bit = first + iter.index * IDA_BITMAP_BITS;
+ return true;
+}
+EXPORT_SYMBOL(xb_find_zero);
+
+#ifndef __KERNEL__
+
+static DEFINE_XB(xb1);
+
+static void xbitmap_check_bit(unsigned long bit)
+{
+ unsigned long nbit = 0;
+
+ xb_preload(GFP_KERNEL);
+ assert(!xb_test_bit(&xb1, bit));
+ assert(xb_set_bit(&xb1, bit) == 0);
+ assert(xb_test_bit(&xb1, bit));
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == bit);
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == bit);
+ nbit++;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == bit + 1);
+ xb_clear_bit(&xb1, bit);
+ assert(xb_empty(&xb1));
+ xb_clear_bit(&xb1, bit);
+ assert(xb_empty(&xb1));
+ nbit = 0;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == 0);
+ xb_preload_end();
+}
+
+static void xbitmap_check_bit_range(void)
+{
+ unsigned long nbit = 0;
+
+ /* Regular test1: node = NULL */
+ xb_preload(GFP_KERNEL);
+ xb_set_bit(&xb1, 700);
+ xb_preload_end();
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == 700);
+ nbit++;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == 701);
+ xb_zero(&xb1, 0, 1023);
+
+ /*
+ * Regular test 2
+ * set bit 2000, 2001, 2040
+ * Next 1 in [0, 2048) --> 2000
+ * Next 1 in [2000, 2002) --> 2000
+ * Next 1 in [2002, 2041) --> 2040
+ * Next 1 in [2002, 2040) --> none
+ * Next 0 in [2000, 2048) --> 2002
+ * Next 0 in [2048, 2060) --> 2048
+ */
+ xb_preload(GFP_KERNEL);
+ assert(!xb_set_bit(&xb1, 2000));
+ assert(!xb_set_bit(&xb1, 2001));
+ assert(!xb_set_bit(&xb1, 2040));
+ nbit = 0;
+ assert(xb_find_set(&xb1, 2048, &nbit) == true);
+ assert(nbit == 2000);
+ assert(xb_find_set(&xb1, 2002, &nbit) == true);
+ assert(nbit == 2000);
+ nbit = 2002;
+ assert(xb_find_set(&xb1, 2041, &nbit) == true);
+ assert(nbit == 2040);
+ nbit = 2002;
+ assert(xb_find_set(&xb1, 2040, &nbit) == true);
+ assert(nbit == 2040);
+ nbit = 2000;
+ assert(xb_find_zero(&xb1, 2048, &nbit) == true);
+ assert(nbit == 2002);
+ nbit = 2048;
+ assert(xb_find_zero(&xb1, 2060, &nbit) == true);
+ assert(nbit == 2048);
+ xb_zero(&xb1, 0, 2047);
+ nbit = 0;
+ assert(xb_find_set(&xb1, 2048, &nbit) == false);
+ assert(nbit == 0);
+ xb_preload_end();
+
+ /*
+ * Overflow tests:
+ * Set bit 1 and ULONG_MAX - 4
+ * Next 1 in [ULONG_MAX - 4, ULONG_MAX) --> ULONG_MAX - 4
+ * Next 1 [ULONG_MAX - 3, ULONG_MAX + 4) --> none
+ * Next 0 [ULONG_MAX - 4, ULONG_MAX + 4) --> none
+ */
+ xb_preload(GFP_KERNEL);
+ assert(!xb_set_bit(&xb1, 1));
+ xb_preload_end();
+ xb_preload(GFP_KERNEL);
+ assert(!xb_set_bit(&xb1, ULONG_MAX - 4));
+ nbit = ULONG_MAX - 4;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == ULONG_MAX - 4);
+ nbit++;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == ULONG_MAX - 3);
+ nbit--;
+ assert(xb_find_zero(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == ULONG_MAX - 3);
+ xb_zero(&xb1, ULONG_MAX - 4, ULONG_MAX);
+ nbit = ULONG_MAX - 10;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == ULONG_MAX - 10);
+ xb_zero(&xb1, 0, 1);
+ nbit = 0;
+ assert(xb_find_set(&xb1, 2, &nbit) == false);
+ assert(nbit == 0);
+ xb_preload_end();
+}
+
+/* Check that setting an already-full bitmap works */
+static void xbitmap_check_set(unsigned long base)
+{
+ unsigned long i;
+
+ assert(xb_empty(&xb1));
+
+ for (i = 0; i < 64 * 1024; i++) {
+ xb_preload(GFP_KERNEL);
+ assert(xb_set_bit(&xb1, base + i) == 0);
+ xb_preload_end();
+ }
+ for (i = 0; i < 64 * 1024; i++)
+ assert(xb_set_bit(&xb1, base + i) == 0);
+
+ for (i = 0; i < 64 * 1024; i++)
+ xb_clear_bit(&xb1, base + i);
+
+ assert(xb_empty(&xb1));
+}
+
+static void xbitmap_checks(void)
+{
+ xb_init(&xb1);
+ xbitmap_check_bit(0);
+ xbitmap_check_bit(30);
+ xbitmap_check_bit(31);
+ xbitmap_check_bit(62);
+ xbitmap_check_bit(63);
+ xbitmap_check_bit(64);
+ xbitmap_check_bit(700);
+ xbitmap_check_bit(1023);
+ xbitmap_check_bit(1024);
+ xbitmap_check_bit(1025);
+ xbitmap_check_bit((1UL << 63) | (1UL << 24));
+ xbitmap_check_bit((1UL << 63) | (1UL << 24) | 70);
+
+ xbitmap_check_bit_range();
+
+ xbitmap_check_set(0);
+ xbitmap_check_set(1024);
+ xbitmap_check_set(1024 * 64);
+}
+
+int __weak main(void)
+{
+ radix_tree_init();
+ xbitmap_checks();
+}
+#endif
diff --git a/tools/include/linux/bitmap.h b/tools/include/linux/bitmap.h
index ca160270fdfa..6b559ee25def 100644
--- a/tools/include/linux/bitmap.h
+++ b/tools/include/linux/bitmap.h
@@ -37,6 +37,40 @@ static inline void bitmap_zero(unsigned long *dst, int nbits)
}
}
+static inline void __bitmap_clear(unsigned long *map, unsigned int start,
+ int len)
+{
+ unsigned long *p = map + BIT_WORD(start);
+ const unsigned int size = start + len;
+ int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
+ unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
+
+ while (len - bits_to_clear >= 0) {
+ *p &= ~mask_to_clear;
+ len -= bits_to_clear;
+ bits_to_clear = BITS_PER_LONG;
+ mask_to_clear = ~0UL;
+ p++;
+ }
+ if (len) {
+ mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
+ *p &= ~mask_to_clear;
+ }
+}
+
+static inline __always_inline void bitmap_clear(unsigned long *map,
+ unsigned int start,
+ unsigned int nbits)
+{
+ if (__builtin_constant_p(nbits) && nbits == 1)
+ __clear_bit(start, map);
+ else if (__builtin_constant_p(start) && IS_ALIGNED(start, 8) &&
+ __builtin_constant_p(nbits) && IS_ALIGNED(nbits, 8))
+ memset((char *)map + start / 8, 0, nbits / 8);
+ else
+ __bitmap_clear(map, start, nbits);
+}
+
static inline void bitmap_fill(unsigned long *dst, unsigned int nbits)
{
unsigned int nlongs = BITS_TO_LONGS(nbits);
diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h
index 0ad884452c5c..3c992ae15440 100644
--- a/tools/include/linux/kernel.h
+++ b/tools/include/linux/kernel.h
@@ -13,6 +13,8 @@
#define UINT_MAX (~0U)
#endif
+#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
+
#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
#define PERF_ALIGN(x, a) __PERF_ALIGN_MASK(x, (typeof(x))(a)-1)
diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile
index fa7ee369b3c9..adf36e34dd77 100644
--- a/tools/testing/radix-tree/Makefile
+++ b/tools/testing/radix-tree/Makefile
@@ -1,12 +1,13 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS += -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address
-LDFLAGS += -fsanitize=address
-LDLIBS+= -lpthread -lurcu
-TARGETS = main idr-test multiorder
+LDFLAGS += -fsanitize=address $(LDLIBS)
+LDLIBS := -lpthread -lurcu
+TARGETS = main idr-test multiorder xbitmap
CORE_OFILES := radix-tree.o idr.o linux.o test.o find_bit.o
OFILES = main.o $(CORE_OFILES) regression1.o regression2.o regression3.o \
- tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o
+ tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o \
+ xbitmap.o
ifndef SHIFT
SHIFT=3
@@ -25,8 +26,11 @@ idr-test: idr-test.o $(CORE_OFILES)
multiorder: multiorder.o $(CORE_OFILES)
+xbitmap: xbitmap.o $(CORE_OFILES)
+ $(CC) $(CFLAGS) $(LDFLAGS) $^ -o xbitmap
+
clean:
- $(RM) $(TARGETS) *.o radix-tree.c idr.c generated/map-shift.h
+ $(RM) $(TARGETS) *.o radix-tree.c idr.c xbitmap.c generated/map-shift.h
vpath %.c ../../lib
@@ -34,6 +38,7 @@ $(OFILES): Makefile *.h */*.h generated/map-shift.h \
../../include/linux/*.h \
../../include/asm/*.h \
../../../include/linux/radix-tree.h \
+ ../../../include/linux/xbitmap.h \
../../../include/linux/idr.h
radix-tree.c: ../../../lib/radix-tree.c
@@ -42,6 +47,9 @@ radix-tree.c: ../../../lib/radix-tree.c
idr.c: ../../../lib/idr.c
sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < $< > $@
+xbitmap.c: ../../../lib/xbitmap.c
+ sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < $< > $@
+
.PHONY: mapshift
mapshift:
diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index c3bc3f364f68..426f32f28547 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -17,6 +17,4 @@
#define pr_debug printk
#define pr_cont printk
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
-
#endif /* _KERNEL_H */
diff --git a/tools/testing/radix-tree/linux/xbitmap.h b/tools/testing/radix-tree/linux/xbitmap.h
new file mode 100644
index 000000000000..61de214542e7
--- /dev/null
+++ b/tools/testing/radix-tree/linux/xbitmap.h
@@ -0,0 +1 @@
+#include "../../../../include/linux/xbitmap.h"
diff --git a/tools/testing/radix-tree/main.c b/tools/testing/radix-tree/main.c
index 257f3f8aacaa..d112363262ae 100644
--- a/tools/testing/radix-tree/main.c
+++ b/tools/testing/radix-tree/main.c
@@ -326,6 +326,10 @@ static void single_thread_tests(bool long_run)
rcu_barrier();
printv(2, "after idr_checks: %d allocated, preempt %d\n",
nr_allocated, preempt_count);
+ xbitmap_checks();
+ rcu_barrier();
+ printv(2, "after xbitmap_checks: %d allocated, preempt %d\n",
+ nr_allocated, preempt_count);
big_gang_check(long_run);
rcu_barrier();
printv(2, "after big_gang_check: %d allocated, preempt %d\n",
diff --git a/tools/testing/radix-tree/test.h b/tools/testing/radix-tree/test.h
index d9c031dbeb1a..8175d6b23b32 100644
--- a/tools/testing/radix-tree/test.h
+++ b/tools/testing/radix-tree/test.h
@@ -38,6 +38,7 @@ void benchmark(void);
void idr_checks(void);
void ida_checks(void);
void ida_thread_tests(void);
+void xbitmap_checks(void);
struct item *
item_tag_set(struct radix_tree_root *root, unsigned long index, int tag);
^ permalink raw reply related
* Question about eliminating virtio-scsi 30s timeout and hot-unplug
From: Jason Behmer via Virtualization @ 2017-12-21 19:29 UTC (permalink / raw)
To: pbonzini; +Cc: virtualization
[-- Attachment #1.1: Type: text/plain, Size: 418 bytes --]
Hi Paolo,
I had a question about this patch that eliminates virtio-scsi timeout on
IOs - https://patchwork.kernel.org/patch/9802047/. How does this work when
we have IOs outstanding to a device that is then hot-unplugged. I'm under
the impression that those IOs will never get returned with any status
(please correct me if I'm wrong), and we will then end up waiting on them
forever with this patch.
Thanks,
Jason
[-- Attachment #1.2: Type: text/html, Size: 556 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] virtio: make VIRTIO a menuconfig to ease disabling it all
From: Vincent Legoll @ 2017-12-21 16:23 UTC (permalink / raw)
To: Michael Ellerman; +Cc: virtualization, Linux Kernel ML, mst
In-Reply-To: <87tvwkfj2f.fsf@concordia.ellerman.id.au>
Hello,
thanks for the help, and sorry for the poor patch,
On Thu, Dec 21, 2017 at 11:53 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> This breaks all existing .configs *and* defconfigs that use VIRTIO.
>
> Please don't do that.
>
> If you make it default y then everything will continue to work.
Do you want me to respin it with the added default ?
Tchuss
--
Vincent Legoll
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Matthew Wilcox @ 2017-12-21 14:18 UTC (permalink / raw)
To: Wei Wang
Cc: virtio-dev, kvm, mst, qemu-devel, penguin-kernel, mawilcox,
linux-kernel, virtualization, linux-mm, akpm, mhocko
In-Reply-To: <1513823406-43632-1-git-send-email-wei.w.wang@intel.com>
First of all, the test-suite doesn't build, so I don't know whether you
ran it or not. Then I added the xb_find_set() call below, and it fails
the assert, so you should probably fix that.
diff --git a/lib/xbitmap.c b/lib/xbitmap.c
index f03a0f9f9e29..b29af08a7597 100644
--- a/lib/xbitmap.c
+++ b/lib/xbitmap.c
@@ -249,11 +249,12 @@ void xbitmap_check_bit(unsigned long bit)
assert(!xb_test_bit(&xb1, bit));
assert(xb_set_bit(&xb1, bit) == 0);
assert(xb_test_bit(&xb1, bit));
- assert(xb_clear_bit(&xb1, bit) == 0);
+ xb_clear_bit(&xb1, bit);
assert(xb_empty(&xb1));
- assert(xb_clear_bit(&xb1, bit) == 0);
+ xb_clear_bit(&xb1, bit);
assert(xb_empty(&xb1));
xb_preload_end();
+ assert(xb_find_set(&xb1, ULONG_MAX, 0) == bit);
}
static void xbitmap_check_bit_range(void)
diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile
index 34ece7883629..adf36e34dd77 100644
--- a/tools/testing/radix-tree/Makefile
+++ b/tools/testing/radix-tree/Makefile
@@ -1,9 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS += -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address
-LDFLAGS += -fsanitize=address
-LDLIBS+= -lpthread -lurcu
-TARGETS = main idr-test multiorder
+LDFLAGS += -fsanitize=address $(LDLIBS)
+LDLIBS := -lpthread -lurcu
+TARGETS = main idr-test multiorder xbitmap
CORE_OFILES := radix-tree.o idr.o linux.o test.o find_bit.o
OFILES = main.o $(CORE_OFILES) regression1.o regression2.o regression3.o \
tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o \
On Thu, Dec 21, 2017 at 10:30:06AM +0800, Wei Wang wrote:
> v20 RESEND Changes:
> - fixed the !node path
> - added the test cases for the !node path
> - change __builtin_constant_p(start & 7) to __builtin_constant_p(start)
Why would you do such a thing? Just copy the kernel definitions.
> diff --git a/include/linux/xbitmap.h b/include/linux/xbitmap.h
> index 108f929..ede1029 100644
> --- a/include/linux/xbitmap.h
> +++ b/include/linux/xbitmap.h
> @@ -35,6 +35,12 @@ static inline void xb_init(struct xb *xb)
> int xb_set_bit(struct xb *xb, unsigned long bit);
> bool xb_test_bit(const struct xb *xb, unsigned long bit);
> void xb_clear_bit(struct xb *xb, unsigned long bit);
> +void xb_clear_bit_range(struct xb *xb, unsigned long start,
> + unsigned long nbits);
This is xb_zero(). I thought we talked about this before?
> +unsigned long xb_find_set(struct xb *xb, unsigned long size,
> + unsigned long offset);
> +unsigned long xb_find_zero(struct xb *xb, unsigned long size,
> + unsigned long offset);
Since you're using xb_find_zero(), I think we need the tags from the IDR.
At that point, I'm not sure there's a point in keeping the xbitmap and the
IDR as separate data structures.
> +/**
> + * xb_find_set - find the next set bit in a range of bits
> + * @xb: the xbitmap to search from
> + * @offset: the offset in the range to start searching
> + * @size: the size of the range
> + *
> + * Returns: the found bit or, @size if no set bit is found.
> + */
> +unsigned long xb_find_set(struct xb *xb, unsigned long size,
> + unsigned long offset)
> +{
> + struct radix_tree_root *root = &xb->xbrt;
> + struct radix_tree_node *node;
> + void __rcu **slot;
> + struct ida_bitmap *bitmap;
> + unsigned long index = offset / IDA_BITMAP_BITS;
> + unsigned long index_end = size / IDA_BITMAP_BITS;
> + unsigned long bit = offset % IDA_BITMAP_BITS;
> +
> + if (unlikely(offset >= size))
> + return size;
> +
> + while (index <= index_end) {
> + unsigned long ret;
> + unsigned int nbits = size - index * IDA_BITMAP_BITS;
> +
> + bitmap = __radix_tree_lookup(root, index, &node, &slot);
> +
> + if (!node && !bitmap)
> + return size;
> +
> + if (bitmap) {
> + if (nbits > IDA_BITMAP_BITS)
> + nbits = IDA_BITMAP_BITS;
> +
> + ret = find_next_bit(bitmap->bitmap, nbits, bit);
> + if (ret != nbits)
> + return ret + index * IDA_BITMAP_BITS;
> + }
> + bit = 0;
> + index++;
> + }
> +
> + return size;
> +}
> +EXPORT_SYMBOL(xb_find_set);
This is going to be slower than the implementation I sent yesterday. If I
call:
xb_init(xb);
xb_set_bit(xb, ULONG_MAX);
xb_find_set(xb, ULONG_MAX, 0);
it's going to call __radix_tree_lookup() 16 quadrillion times.
My implementation will walk the tree precisely once.
^ permalink raw reply related
* [PATCH v5 4/4] virtio_remoteproc: correct put_device virtio_device.dev
From: weiping zhang @ 2017-12-21 12:40 UTC (permalink / raw)
To: cohuck, ohad, bjorn.andersson, mst, jasowang; +Cc: virtualization
rproc_virtio_dev_release will be called iff virtio_device.dev's
reference count drops to 0. Here we just put vdev.dev, and then
rproc->dev's cleanup will be done in rproc_virtio_dev_release.
Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
---
drivers/remoteproc/remoteproc_virtio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 2946348..b0633fd 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -327,7 +327,7 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
ret = register_virtio_device(vdev);
if (ret) {
- put_device(&rproc->dev);
+ put_device(&vdev->dev);
dev_err(dev, "failed to register vdev: %d\n", ret);
goto out;
}
--
2.9.4
^ permalink raw reply related
* [PATCH v5 3/4] virtio_vop: don't kfree device on register failure
From: weiping zhang @ 2017-12-21 12:40 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
As mentioned at drivers/base/core.c:
/*
* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up the
* reference initialized in this function instead.
*/
so we don't free vdev until vdev->vdev.dev.release be called.
Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/misc/mic/vop/vop_main.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c
index a341938..3633202 100644
--- a/drivers/misc/mic/vop/vop_main.c
+++ b/drivers/misc/mic/vop/vop_main.c
@@ -452,10 +452,12 @@ static irqreturn_t vop_virtio_intr_handler(int irq, void *data)
static void vop_virtio_release_dev(struct device *_d)
{
- /*
- * No need for a release method similar to virtio PCI.
- * Provide an empty one to avoid getting a warning from core.
- */
+ struct virtio_device *vdev =
+ container_of(_d, struct virtio_device, dev);
+ struct _vop_vdev *vop_vdev =
+ container_of(vdev, struct _vop_vdev, vdev);
+
+ kfree(vop_vdev);
}
/*
@@ -466,7 +468,7 @@ static int _vop_add_device(struct mic_device_desc __iomem *d,
unsigned int offset, struct vop_device *vpdev,
int dnode)
{
- struct _vop_vdev *vdev;
+ struct _vop_vdev *vdev, *reg_dev = NULL;
int ret;
u8 type = ioread8(&d->type);
@@ -497,6 +499,7 @@ static int _vop_add_device(struct mic_device_desc __iomem *d,
vdev->c2h_vdev_db = ioread8(&vdev->dc->c2h_vdev_db);
ret = register_virtio_device(&vdev->vdev);
+ reg_dev = vdev;
if (ret) {
dev_err(_vop_dev(vdev),
"Failed to register vop device %u type %u\n",
@@ -512,7 +515,10 @@ static int _vop_add_device(struct mic_device_desc __iomem *d,
free_irq:
vpdev->hw_ops->free_irq(vpdev, vdev->virtio_cookie, vdev);
kfree:
- kfree(vdev);
+ if (reg_dev)
+ put_device(&vdev->vdev.dev);
+ else
+ kfree(vdev);
return ret;
}
@@ -568,7 +574,7 @@ static int _vop_remove_device(struct mic_device_desc __iomem *d,
iowrite8(-1, &dc->h2c_vdev_db);
if (status & VIRTIO_CONFIG_S_DRIVER_OK)
wait_for_completion(&vdev->reset_done);
- kfree(vdev);
+ put_device(&vdev->vdev.dev);
iowrite8(1, &dc->guest_ack);
dev_dbg(&vpdev->dev, "%s %d guest_ack %d\n",
__func__, __LINE__, ioread8(&dc->guest_ack));
--
2.9.4
^ permalink raw reply related
* [PATCH v5 2/4] virtio_pci: don't kfree device on register failure
From: weiping zhang @ 2017-12-21 12:40 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
As mentioned at drivers/base/core.c:
/*
* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up the
* reference initialized in this function instead.
*/
so we don't free vp_dev until vp_dev->vdev.dev.release be called.
Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/virtio/virtio_pci_common.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 1c4797e..48d4d1c 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -513,7 +513,7 @@ static void virtio_pci_release_dev(struct device *_d)
static int virtio_pci_probe(struct pci_dev *pci_dev,
const struct pci_device_id *id)
{
- struct virtio_pci_device *vp_dev;
+ struct virtio_pci_device *vp_dev, *reg_dev = NULL;
int rc;
/* allocate our structure and fill it out */
@@ -551,6 +551,7 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
pci_set_master(pci_dev);
rc = register_virtio_device(&vp_dev->vdev);
+ reg_dev = vp_dev;
if (rc)
goto err_register;
@@ -564,7 +565,10 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
err_probe:
pci_disable_device(pci_dev);
err_enable_device:
- kfree(vp_dev);
+ if (reg_dev)
+ put_device(&vp_dev->vdev.dev);
+ else
+ kfree(vp_dev);
return rc;
}
--
2.9.4
^ permalink raw reply related
* [PATCH v5 1/4] virtio: split device_register into device_initialize and device_add
From: weiping zhang @ 2017-12-21 12:39 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
In order to make caller do a simple cleanup, we split device_register
into device_initialize and device_add. device_initialize always succeeds,
so the caller can always use put_device when register_virtio_device faild.
Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
Suggested-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/virtio/virtio.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index bf7ff39..59e36ef 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -303,11 +303,21 @@ void unregister_virtio_driver(struct virtio_driver *driver)
}
EXPORT_SYMBOL_GPL(unregister_virtio_driver);
+/**
+ * register_virtio_device - register virtio device
+ * @dev : virtio device to be registered
+ *
+ * On error, the caller must call put_device on &@dev->dev (and not kfree),
+ * as another code path may have obtained a reference to @dev.
+ *
+ * Returns: 0 on suceess, -error on failure
+ */
int register_virtio_device(struct virtio_device *dev)
{
int err;
dev->dev.bus = &virtio_bus;
+ device_initialize(&dev->dev);
/* Assign a unique device index and hence name. */
err = ida_simple_get(&virtio_index_ida, 0, 0, GFP_KERNEL);
@@ -330,9 +340,11 @@ int register_virtio_device(struct virtio_device *dev)
INIT_LIST_HEAD(&dev->vqs);
- /* device_register() causes the bus infrastructure to look for a
- * matching driver. */
- err = device_register(&dev->dev);
+ /*
+ * device_add() causes the bus infrastructure to look for a matching
+ * driver.
+ */
+ err = device_add(&dev->dev);
if (err)
ida_simple_remove(&virtio_index_ida, dev->index);
out:
--
2.9.4
^ permalink raw reply related
* [PATCH v5 0/4] use put_device to cleanup resource
From: weiping zhang @ 2017-12-21 12:38 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
Hi,
The main change is split device_register into 2 sperate calls:
device_initalize() and device_add, and then the caller can use
put_device safety when fail to register_virtio_device.
v4->v5:
* virtio: correct some comments
* virtio_remoteproc: use put_device directly, not use temp reg_dev
v3->v4:
* split device_register into device_initialize and devicea_add that
the caller can always use put_device when fail to register virtio
device.
v2->v3:
* virtio: add new helper do get device's status then determine use
put_device or kfree.
v1->v2:
* virtio_pci: add comments in commit message for why using put_device
* virtio_vop: also use put_device int _vop_remove_device
weiping zhang (4):
virtio: split device_register into device_initialize and device_add
virtio_pci: don't kfree device on register failure
virtio_vop: don't kfree device on register failure
virtio_remoteproc: correct put_device virtio_device.dev
drivers/misc/mic/vop/vop_main.c | 20 +++++++++++++-------
drivers/remoteproc/remoteproc_virtio.c | 2 +-
drivers/virtio/virtio.c | 18 +++++++++++++++---
drivers/virtio/virtio_pci_common.c | 8 ++++++--
4 files changed, 35 insertions(+), 13 deletions(-)
--
2.9.4
^ permalink raw reply
* Re: [PATCH v20 0/7] Virtio-balloon Enhancement
From: Matthew Wilcox @ 2017-12-21 12:14 UTC (permalink / raw)
To: Wei Wang
Cc: yang.zhang.wz@gmail.com, kvm@vger.kernel.org, mst@redhat.com,
Tetsuo Handa, liliang.opensource@gmail.com, qemu-devel@nongnu.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
mawilcox@microsoft.com, nilal@redhat.com, riel@redhat.com,
cornelia.huck@de.ibm.com, mhocko@kernel.org, quan.xu0@gmail.com,
linux-kernel@vger.kernel.org, amit.sh
In-Reply-To: <5A3B2148.8050306@intel.com>
On Thu, Dec 21, 2017 at 10:49:44AM +0800, Wei Wang wrote:
> On 12/21/2017 01:10 AM, Matthew Wilcox wrote:
> One more question is about the return value, why would it be ambiguous? I
> think it is the same as find_next_bit() which returns the found bit or size
> if not found.
Because find_next_bit doesn't reasonably support a bitmap which is
ULONG_MAX in size. The point of XBitmap is to support a bitmap which
is ULONG_MAX in size, so every possible return value is a legitimate
"we found a bit here". There's no value which can possibly be used for
"no bit was found".
^ permalink raw reply
* Re: [PATCH] virtio: make VIRTIO a menuconfig to ease disabling it all
From: Michael Ellerman @ 2017-12-21 10:53 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: Vincent Legoll
In-Reply-To: <20171209152657.4762-1-vincent.legoll@gmail.com>
Vincent Legoll <vincent.legoll@gmail.com> writes:
> No need to get into the submenu to disable all VIRTIO-related
> config entries.
>
> This makes it easier to disable all VIRTIO config options
> without entering the submenu. It will also enable one
> to see that en/dis-abled state from the outside menu.
>
> This is only intended to change menuconfig UI, not change
> the config dependencies.
>
> Signed-off-by: Vincent Legoll <vincent.legoll@gmail.com>
> ---
> drivers/virtio/Kconfig | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index cff773f15b7e..d485a63a8233 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -5,7 +5,10 @@ config VIRTIO
> bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
> or CONFIG_S390_GUEST.
>
> -menu "Virtio drivers"
> +menuconfig VIRTIO_MENU
> + bool "Virtio drivers"
> +
> +if VIRTIO_MENU
This breaks all existing .configs *and* defconfigs that use VIRTIO.
Please don't do that.
If you make it default y then everything will continue to work.
cheers
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index d485a63a8233..35897649c24f 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -7,6 +7,7 @@ config VIRTIO
menuconfig VIRTIO_MENU
bool "Virtio drivers"
+ default y
if VIRTIO_MENU
^ permalink raw reply related
* Re: virtio: make VIRTIO a menuconfig to ease disabling it all
From: Andrei Vagin @ 2017-12-21 6:44 UTC (permalink / raw)
To: Vincent Legoll; +Cc: virtualization, linux-kernel, mst
In-Reply-To: <20171209152657.4762-1-vincent.legoll@gmail.com>
On Sat, Dec 09, 2017 at 04:26:57PM +0100, Vincent Legoll wrote:
> No need to get into the submenu to disable all VIRTIO-related
> config entries.
>
> This makes it easier to disable all VIRTIO config options
> without entering the submenu. It will also enable one
> to see that en/dis-abled state from the outside menu.
>
> This is only intended to change menuconfig UI, not change
> the config dependencies.
>
> Signed-off-by: Vincent Legoll <vincent.legoll@gmail.com>
> ---
> drivers/virtio/Kconfig | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index cff773f15b7e..d485a63a8233 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -5,7 +5,10 @@ config VIRTIO
> bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
> or CONFIG_S390_GUEST.
>
> -menu "Virtio drivers"
> +menuconfig VIRTIO_MENU
> + bool "Virtio drivers"
Hi Vincent,
make localyesconfig and make localmodconfig doesn't work with this
patch.
My scenario looks like this:
* Create a virtual machine with Ubuntu 14.04 in GCE
* git clone git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
* cd linux-next
* curl -o .config https://raw.githubusercontent.com/avagin/criu/linux-next/scripts/linux-next-config
* make localyesconfig
Without this patch:
$ cat .config | grep VIRTIO
CONFIG_BLK_MQ_VIRTIO=y
CONFIG_VIRTIO_BLK=y
# CONFIG_VIRTIO_BLK_SCSI is not set
CONFIG_SCSI_VIRTIO=y
CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_CONSOLE=y
# CONFIG_HW_RANDOM_VIRTIO is not set
CONFIG_VIRTIO=y
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_PCI_LEGACY=y
CONFIG_VIRTIO_BALLOON=y
# CONFIG_VIRTIO_INPUT is not set
CONFIG_VIRTIO_MMIO=y
CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
# CONFIG_RPMSG_VIRTIO is not set
# CONFIG_CRYPTO_DEV_VIRTIO is not set
With this patch:
$ cat .config | grep VIRTIO
CONFIG_BLK_MQ_VIRTIO=y
# CONFIG_VIRTIO_BLK is not set
CONFIG_SCSI_VIRTIO=y
# CONFIG_VIRTIO_NET is not set
CONFIG_CAIF_VIRTIO=y
# CONFIG_VIRTIO_CONSOLE is not set
# CONFIG_HW_RANDOM_VIRTIO is not set
CONFIG_VIRTIO=y
# CONFIG_VIRTIO_MENU is not set
# CONFIG_RPMSG_VIRTIO is not set
# CONFIG_CRYPTO_DEV_VIRTIO is not set
You can see that with this patch CONFIG_VIRTIO_BLK is not set.
It is wrong, and the kernel with this config will not be able to boot.
We can add "default y" to fix this problem.
https://travis-ci.org/avagin/linux/jobs/313348334
https://travis-ci.org/avagin/linux/jobs/318491188
Thanks,
Andrei
> +
> +if VIRTIO_MENU
>
> config VIRTIO_PCI
> tristate "PCI driver for virtio devices"
> @@ -79,4 +82,4 @@ config VIRTIO_MMIO_CMDLINE_DEVICES
>
> If unsure, say 'N'.
>
> -endmenu
> +endif # VIRTIO_MENU
^ permalink raw reply
* Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available
From: Jakub Kicinski @ 2017-12-21 4:52 UTC (permalink / raw)
To: Siwei Liu
Cc: mst, sridhar.samudrala, alexander.duyck, virtualization, netdev,
David Miller
In-Reply-To: <CADGSJ22mDyC0Sw2EPW96ZWvMPLnzif+Y9MMouhM-6T03ZaKA0Q@mail.gmail.com>
On Wed, 20 Dec 2017 18:16:30 -0800, Siwei Liu wrote:
> > The plan is to remove the delay and do the naming in the kernel.
> > This was suggested by Lennart since udev is only doing naming policy
> > because kernel names were not repeatable.
> >
> > This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.
> >
> > Patch is pending.
>
> While it's good to show VF with specific naming to indicate
> enslavement, I wonder wouldn't it be better to hide this netdev at all
> from the user space? IMHO this extra device is useless when being
> enslaved and we may delegate controls (e.g. ethtool) over to the
> para-virtual device instead? That way it's possible to eliminate the
> possibility of additional udev setup or modification?
>
> I'm not sure if this is consistent with Windows guest or not, but I
> don't find it _Linux_ user friendly that ethtool doesn't work on the
> composite interface any more, and I have to end up with finding out
> the correct enslaved VF I must operate on.
Hiding "low level" netdevs comes up from time to time, and is more
widely applicable than just to VF bonds. We should find a generic
solution to that problem.
^ permalink raw reply
* Re: [PATCH v4 4/4] virtio_remoteproc: don't kfree device on register failure
From: weiping zhang @ 2017-12-21 3:08 UTC (permalink / raw)
To: Cornelia Huck; +Cc: virtualization, Michael S . Tsirkin
In-Reply-To: <20171220171201.6aa2b68f.cohuck@redhat.com>
2017-12-21 0:12 GMT+08:00 Cornelia Huck <cohuck@redhat.com>:
> On Wed, 20 Dec 2017 12:27:33 +0800
> weiping zhang <zwp10758@gmail.com> wrote:
>
>> rproc_virtio_dev_release will be called iff virtio_device.dev's
>> refer count became to 0. Here we should check if we call device_register
>
> "reference count drops to 0"
>
> s/call/called/
rproc_add_virtio_dev is not like other virtio driver, it always call
register_virtio_device,
so commit message would be:
virtio_remoteproc: correct put_device virtio_device.dev
rproc_virtio_dev_release will be called iff virtio_device.dev's
reference count drops to 0. Here we just put vdev.dev, and then
rproc->dev's cleanup will be done in rproc_virtio_dev_release.
>> or not, if called, put vdev.dev, and then rproc->dev's cleanup will be
>> done in rproc_virtio_dev_release, otherwise we do cleanup directly.
>>
>> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
>> ---
>> drivers/remoteproc/remoteproc_virtio.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
>> index 2946348..1073ea3 100644
>> --- a/drivers/remoteproc/remoteproc_virtio.c
>> +++ b/drivers/remoteproc/remoteproc_virtio.c
>> @@ -304,7 +304,7 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> {
>> struct rproc *rproc = rvdev->rproc;
>> struct device *dev = &rproc->dev;
>> - struct virtio_device *vdev = &rvdev->vdev;
>> + struct virtio_device *vdev = &rvdev->vdev, *reg_dev = NULL;
>> int ret;
>>
>> vdev->id.device = id,
>> @@ -326,15 +326,24 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> kref_get(&rvdev->refcount);
>>
>> ret = register_virtio_device(vdev);
>> + reg_dev = vdev;
>> if (ret) {
>> - put_device(&rproc->dev);
>> dev_err(dev, "failed to register vdev: %d\n", ret);
>> goto out;
>> }
>>
>> dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev), id);
>>
>> + return 0;
>> +
>> out:
>> + if (reg_dev)
>> + put_device(&vdev->dev);
>> + else {
>> + kref_put(&rvdev->refcount, rproc_vdev_release);
>> + put_device(&rproc->dev);
>> + }
>> +
>> return ret;
>> }
>>
>
> I think in this case using the marker makes a straightforward cleanup
> way too complicated. There's a single way we can get to the out label,
> and that's when register_virtio_device() failed. Switching
> put_device(&rproc->dev) to put_device(@vdev->dev) (what your first
> patch did) seems like the way to go.
OK, put it directly at v5.
> (It also may be good to cc: the maintainers for this driver.)
OK, thanks
^ permalink raw reply
* Re: [PATCH v20 0/7] Virtio-balloon Enhancement
From: Wei Wang @ 2017-12-21 2:49 UTC (permalink / raw)
To: Matthew Wilcox
Cc: yang.zhang.wz@gmail.com, kvm@vger.kernel.org, mst@redhat.com,
Tetsuo Handa, liliang.opensource@gmail.com, qemu-devel@nongnu.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
mawilcox@microsoft.com, nilal@redhat.com, riel@redhat.com,
cornelia.huck@de.ibm.com, mhocko@kernel.org, quan.xu0@gmail.com,
linux-kernel@vger.kernel.org, amit.sh
In-Reply-To: <20171220171019.GA12236@bombadil.infradead.org>
On 12/21/2017 01:10 AM, Matthew Wilcox wrote:
> On Wed, Dec 20, 2017 at 04:13:16PM +0000, Wang, Wei W wrote:
>> On Wednesday, December 20, 2017 8:26 PM, Matthew Wilcox wrote:
>>> unsigned long bit;
>>> xb_preload(GFP_KERNEL);
>>> xb_set_bit(xb, 700);
>>> xb_preload_end();
>>> bit = xb_find_set(xb, ULONG_MAX, 0);
>>> assert(bit == 700);
>> This above test will result in "!node with bitmap !=NULL", and it goes to the regular "if (bitmap)" path, which finds 700.
>>
>> A better test would be
>> ...
>> xb_set_bit(xb, 700);
>> assert(xb_find_set(xb, ULONG_MAX, 800) == ULONG_MAX);
>> ...
> I decided to write a test case to show you what I meant, then I discovered
> the test suite didn't build, then the test I wrote took forever to run, so
> I rewrote xb_find_set() using the radix tree iterators. So I have no idea
> what bugs may be in your implementation, but at least this function passes
> the current test suite. Of course, there may be gaps in the test suite.
> And since I changed the API to not have the ambiguous return value, I
> also changed the test suite, and maybe I introduced a bug.
Thanks for the effort. That's actually caused by the previous "!node"
path, which incorrectly changed "index = (index | RADIX_TREE_MAP_MASK) +
1". With the change below, it will run pretty well with the test cases.
if (!node && !bitmap)
return size;
Would you mind to have a try with the v20 RESEND patch that was just
shared? It makes the above change and added the test case you suggested?
One more question is about the return value, why would it be ambiguous?
I think it is the same as find_next_bit() which returns the found bit or
size if not found.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v4 1/4] virtio: split device_register into device_initialize and device_add
From: weiping zhang @ 2017-12-21 2:43 UTC (permalink / raw)
To: Cornelia Huck; +Cc: virtualization, Michael S . Tsirkin
In-Reply-To: <CAA70yB4e6Qp_2D+6-RCSVLKvSVxJ_vSeBi8Day7b1pCOzvfD_g@mail.gmail.com>
2017-12-21 10:37 GMT+08:00 weiping zhang <zwp10758@gmail.com>:
> 2017-12-20 23:53 GMT+08:00 Cornelia Huck <cohuck@redhat.com>:
>> On Wed, 20 Dec 2017 12:26:25 +0800
>> weiping zhang <zwp10758@gmail.com> wrote:
>>
>> [you used a different mail address in your From: than in your s-o-b:;
>> same for the other patches]
a wrong setting of my email client, correct it next time, thanks.
>>> In order to make caller do a simple cleanup, we split device_register
>>> into device_initialize and device_add. device_initialize always sucess,
>>
>> s/success/succeeds/
>>
>>> the caller can always use put_device when fail to register virtio_device
>>
>> "so the caller can always use put_device when register_virtio_device
>> failed,"
>>
>>> no matter fail at ida_simple_get or at device_add.
>>
>> "no matter whether it failed..."
>>
>>> + *
>>> + * If an error occurs, the caller must use put_device, instead of kfree, because
>>> + * device_initialize and device_add will increase @dev->dev's reference count.
>>
>> That's not correct: It's not because of device_add increasing the
>> reference count (it releases it again on failure), but because another
>> code path may have obtained a reference.
>>
>> What about:
>>
>> "On error, the caller must call put_device on &@dev->dev (and not
>> kfree), as another code path may have obtained a reference to @dev."
>>
> It's good to understand, further more dev->name may has a bit mem leak.
> anyway, I'll correct all comments at V5. Thanks very much.
>>> + *
>>> + * Returns: 0 on suceess, -error on failure
>>> + */
>>> int register_virtio_device(struct virtio_device *dev)
^ permalink raw reply
* Re: [PATCH v4 1/4] virtio: split device_register into device_initialize and device_add
From: weiping zhang @ 2017-12-21 2:37 UTC (permalink / raw)
To: Cornelia Huck; +Cc: virtualization, Michael S . Tsirkin
In-Reply-To: <20171220165316.5545b24c.cohuck@redhat.com>
2017-12-20 23:53 GMT+08:00 Cornelia Huck <cohuck@redhat.com>:
> On Wed, 20 Dec 2017 12:26:25 +0800
> weiping zhang <zwp10758@gmail.com> wrote:
>
> [you used a different mail address in your From: than in your s-o-b:;
> same for the other patches]
>
>> In order to make caller do a simple cleanup, we split device_register
>> into device_initialize and device_add. device_initialize always sucess,
>
> s/success/succeeds/
>
>> the caller can always use put_device when fail to register virtio_device
>
> "so the caller can always use put_device when register_virtio_device
> failed,"
>
>> no matter fail at ida_simple_get or at device_add.
>
> "no matter whether it failed..."
>
>> + *
>> + * If an error occurs, the caller must use put_device, instead of kfree, because
>> + * device_initialize and device_add will increase @dev->dev's reference count.
>
> That's not correct: It's not because of device_add increasing the
> reference count (it releases it again on failure), but because another
> code path may have obtained a reference.
>
> What about:
>
> "On error, the caller must call put_device on &@dev->dev (and not
> kfree), as another code path may have obtained a reference to @dev."
>
It's good to understand, further more dev->name may has a bit mem leak.
anyway, I'll correct all comments at V5. Thanks very much.
>> + *
>> + * Returns: 0 on suceess, -error on failure
>> + */
>> int register_virtio_device(struct virtio_device *dev)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox