[PATCH net-next 2/8] skb: api to report errors for zero copy skbs
Orphaning frags for zero copy skbs needs to allocate data in atomic
context so is has a chance to fail. If it does we currently discard
the skb which is safe, but we don't report anything to the caller,
so it can not recover by e.g. disabling zero copy.
Add an API to free skb reporting such errors: this is used
by tun in case orphaning frags fails.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 19 +++
2 files changed, 20 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 8bac11b..0644432 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -568,6 +568,7 @@ static inline struct rtable *skb_rtable(const struct
sk_buff *skb)
}
extern void kfree_skb(struct sk_buff *skb);
+extern void skb_tx_error(struct sk_buff *skb, int err);
extern void consume_skb(struct sk_buff *skb);
extern void __kfree_skb(struct sk_buff *skb);
extern struct kmem_cache *skbuff_head_cache;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index eb31f6e..ad99c64 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -635,6 +635,25 @@ void kfree_skb(struct sk_buff *skb)
EXPORT_SYMBOL(kfree_skb);
/**
+ * kfree_skb_on_error - report an sk_buff xmit error
+ * @skb: buffer that triggered an error
+ *
+ * Report xmit error if a device callback is tracking this skb.
+ */
+void skb_tx_error(struct sk_buff *skb, int err)
+{
+ if (skb_shinfo(skb)-tx_flags SKBTX_DEV_ZEROCOPY) {
+ struct ubuf_info *uarg;
+
+ uarg = skb_shinfo(skb)-destructor_arg;
+ if (uarg-callback)
+ uarg-callback(uarg, err);
+ skb_shinfo(skb)-tx_flags = ~SKBTX_DEV_ZEROCOPY;
+ }
+}
+EXPORT_SYMBOL(skb_tx_error);
+
+/**
* consume_skb - free an skbuff
* @skb: buffer to free
*
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 03/12] VMCI: doorbell implementation.
VMCI doorbell code allows for notifcations between host and guest.
Signed-off-by: George Zhang georgezh...@vmware.com
---
drivers/misc/vmw_vmci/vmci_doorbell.c | 673 +
drivers/misc/vmw_vmci/vmci_doorbell.h | 53 +++
2 files changed, 726 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/vmw_vmci/vmci_doorbell.c
create mode 100644 drivers/misc/vmw_vmci/vmci_doorbell.h
diff --git a/drivers/misc/vmw_vmci/vmci_doorbell.c
b/drivers/misc/vmw_vmci/vmci_doorbell.c
new file mode 100644
index 000..ebe4180
--- /dev/null
+++ b/drivers/misc/vmw_vmci/vmci_doorbell.c
@@ -0,0 +1,673 @@
+/*
+ * VMware VMCI Driver
+ *
+ * Copyright (C) 2012 VMware, Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation version 2 and no later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include linux/vmw_vmci_defs.h
+#include linux/vmw_vmci_api.h
+#include linux/completion.h
+#include linux/hash.h
+#include linux/kernel.h
+#include linux/list.h
+#include linux/module.h
+#include linux/sched.h
+#include linux/slab.h
+
+#include vmci_common_int.h
+#include vmci_datagram.h
+#include vmci_doorbell.h
+#include vmci_resource.h
+#include vmci_driver.h
+#include vmci_route.h
+
+
+#define VMCI_DOORBELL_INDEX_BITS 6
+#define VMCI_DOORBELL_INDEX_TABLE_SIZE (1 VMCI_DOORBELL_INDEX_BITS)
+#define VMCI_DOORBELL_HASH(_idx) hash_32(_idx, VMCI_DOORBELL_INDEX_BITS)
+
+/*
+ * DoorbellEntry describes the a doorbell notification handle allocated by the
+ * host.
+ */
+struct dbell_entry {
+ struct vmci_resource resource;
+ struct hlist_node node;
+ struct work_struct work;
+ vmci_callback notify_cb;
+ void *client_data;
+ u32 idx;
+ u32 priv_flags;
+ bool run_delayed;
+ atomic_t active;/* Only used by guest personality */
+};
+
+/* The VMCI index table keeps track of currently registered doorbells. */
+struct dbell_index_table {
+ spinlock_t lock;/* Index table lock */
+ struct hlist_head entries[VMCI_DOORBELL_INDEX_TABLE_SIZE];
+};
+
+static struct dbell_index_table vmci_doorbell_it = {
+ .lock = __SPIN_LOCK_UNLOCKED(vmci_doorbell_it.lock),
+};
+
+/*
+ * The max_notify_idx is one larger than the currently known bitmap index in
+ * use, and is used to determine how much of the bitmap needs to be scanned.
+ */
+static u32 max_notify_idx;
+
+/*
+ * The notify_idx_count is used for determining whether there are free entries
+ * within the bitmap (if notify_idx_count + 1 max_notify_idx).
+ */
+static u32 notify_idx_count;
+
+/*
+ * The last_notify_idx_reserved is used to track the last index handed out - in
+ * the case where multiple handles share a notification index, we hand out
+ * indexes round robin based on last_notify_idx_reserved.
+ */
+static u32 last_notify_idx_reserved;
+
+/* This is a one entry cache used to by the index allocation. */
+static u32 last_notify_idx_released = PAGE_SIZE;
+
+
+/*
+ * Utility function that retrieves the privilege flags associated
+ * with a given doorbell handle. For guest endpoints, the
+ * privileges are determined by the context ID, but for host
+ * endpoints privileges are associated with the complete
+ * handle. Hypervisor endpoints are not yet supported.
+ */
+int vmci_dbell_get_priv_flags(struct vmci_handle handle, u32 *priv_flags)
+{
+ if (priv_flags == NULL || handle.context == VMCI_INVALID_ID)
+ return VMCI_ERROR_INVALID_ARGS;
+
+ if (handle.context == VMCI_HOST_CONTEXT_ID) {
+ struct dbell_entry *entry;
+ struct vmci_resource *resource;
+
+ resource = vmci_resource_by_handle(handle,
+ VMCI_RESOURCE_TYPE_DOORBELL);
+ if (!resource)
+ return VMCI_ERROR_NOT_FOUND;
+
+ entry = container_of(resource, struct dbell_entry, resource);
+ *priv_flags = entry-priv_flags;
+ vmci_resource_put(resource);
+ } else if (handle.context == VMCI_HYPERVISOR_CONTEXT_ID) {
+ /*
+* Hypervisor endpoints for notifications are not
+* supported (yet).
+*/
+ return VMCI_ERROR_INVALID_ARGS;
+ } else {
+ *priv_flags = vmci_context_get_priv_flags(handle.context);
+ }
+
+ return VMCI_SUCCESS;
+}
+
+/*
+ * Find doorbell entry by bitmap index.
+ */
+static struct dbell_entry *dbell_index_table_find(u32 idx)
+{
+ u32 bucket = VMCI_DOORBELL_HASH(idx);
+ struct dbell_entry *dbell;
+ struct hlist_node
[PATCH 04/12] VMCI: device driver implementaton.
VMCI driver code implementes both the host and guest personalities of the VMCI
driver.
Signed-off-by: George Zhang georgezh...@vmware.com
---
drivers/misc/vmw_vmci/vmci_driver.c | 159 +++
drivers/misc/vmw_vmci/vmci_driver.h | 50 +++
2 files changed, 209 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/vmw_vmci/vmci_driver.c
create mode 100644 drivers/misc/vmw_vmci/vmci_driver.h
diff --git a/drivers/misc/vmw_vmci/vmci_driver.c
b/drivers/misc/vmw_vmci/vmci_driver.c
new file mode 100644
index 000..1ca0c7e
--- /dev/null
+++ b/drivers/misc/vmw_vmci/vmci_driver.c
@@ -0,0 +1,159 @@
+/*
+ * VMware VMCI Driver
+ *
+ * Copyright (C) 2012 VMware, Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation version 2 and no later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include linux/vmw_vmci_defs.h
+#include linux/vmw_vmci_api.h
+#include linux/atomic.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/init.h
+
+#include vmci_driver.h
+#include vmci_event.h
+
+static bool vmci_disable_host;
+module_param_named(disable_host, vmci_disable_host, bool, 0);
+MODULE_PARM_DESC(disable_host,
+Disable driver host personality (default=enabled));
+
+static bool vmci_disable_guest;
+module_param_named(disable_guest, vmci_disable_guest, bool, 0);
+MODULE_PARM_DESC(disable_guest,
+Disable driver guest personality (default=enabled));
+
+static bool vmci_guest_personality_initialized;
+static bool vmci_host_personality_initialized;
+
+/*
+ * vmci_get_context_id() - Gets the current context ID.
+ *
+ * Returns the current context ID. Note that since this is accessed only
+ * from code running in the host, this always returns the host context ID.
+ */
+u32 vmci_get_context_id(void)
+{
+ if (vmci_guest_code_active()) {
+ return vmci_get_vm_context_id();
+ } else if (vmci_host_code_active()) {
+ return VMCI_HOST_CONTEXT_ID;
+ } else {
+ return VMCI_INVALID_ID;
+ }
+}
+EXPORT_SYMBOL_GPL(vmci_get_context_id);
+
+/*
+ * vmci_version() - Returns the version of the driver.
+ *
+ * Returns the version of the VMCI driver.
+ */
+u32 vmci_version(void)
+{
+ return VMCI_VERSION;
+}
+EXPORT_SYMBOL_GPL(vmci_version);
+
+static int __init vmci_core_init(void)
+{
+ int result;
+
+ result = vmci_ctx_init();
+ if (result VMCI_SUCCESS) {
+ pr_err(Failed to initialize VMCIContext (result=%d).\n,
+ result);
+ return -EINVAL;
+ }
+
+ result = vmci_datagram_init();
+ if (result VMCI_SUCCESS) {
+ pr_err(Failed to initialize VMCIDatagram (result=%d).\n,
+ result);
+ return -EINVAL;
+ }
+
+ result = vmci_event_init();
+ if (result VMCI_SUCCESS) {
+ pr_err(Failed to initialize VMCIEvent (result=%d).\n, result);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void __exit vmci_core_exit(void)
+{
+ vmci_event_exit();
+}
+
+static int __init vmci_drv_init(void)
+{
+ int error;
+
+ error = vmci_core_init();
+ if (error)
+ return error;
+
+ if (!vmci_disable_guest) {
+ error = vmci_guest_init();
+ if (error) {
+ pr_warn(Failed to initialize guest personality
(err=%d).\n,
+ error);
+ } else {
+ vmci_guest_personality_initialized = true;
+ pr_info(Guest personality initialized and is %s\n,
+ vmci_guest_code_active() ?
+ active : inactive);
+ }
+ }
+
+ if (!vmci_disable_host) {
+ error = vmci_host_init();
+ if (error) {
+ pr_warn(Unable to initialize host personality
(err=%d).\n,
+ error);
+ } else {
+ vmci_host_personality_initialized = true;
+ pr_info(Initialized host personality\n);
+ }
+ }
+
+ if (!vmci_guest_personality_initialized
+ !vmci_host_personality_initialized) {
+ vmci_core_exit();
+ return -ENODEV;
+ }
+
+ pr_debug(Module is initialized\n);
+ return 0;
+}
+module_init(vmci_drv_init);
+
+static void __exit vmci_drv_exit(void)
+{
+ if (vmci_guest_personality_initialized)
+ vmci_guest_exit();
+
+
[PATCH 06/12] VMCI: handle array implementation.
VMCI handle code adds support for dynamic arrays that will grow if they need to.
Signed-off-by: George Zhang georgezh...@vmware.com
---
drivers/misc/vmw_vmci/vmci_handle_array.c | 162 +
drivers/misc/vmw_vmci/vmci_handle_array.h | 46
2 files changed, 208 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/vmw_vmci/vmci_handle_array.c
create mode 100644 drivers/misc/vmw_vmci/vmci_handle_array.h
diff --git a/drivers/misc/vmw_vmci/vmci_handle_array.c
b/drivers/misc/vmw_vmci/vmci_handle_array.c
new file mode 100644
index 000..c7db831
--- /dev/null
+++ b/drivers/misc/vmw_vmci/vmci_handle_array.c
@@ -0,0 +1,162 @@
+/*
+ * VMware VMCI Driver
+ *
+ * Copyright (C) 2012 VMware, Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation version 2 and no later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include linux/slab.h
+#include vmci_handle_array.h
+
+static size_t handle_arr_calc_size(size_t capacity)
+{
+ return sizeof(struct vmci_handle_arr) +
+ capacity * sizeof(struct vmci_handle);
+}
+
+struct vmci_handle_arr *vmci_handle_arr_create(size_t capacity)
+{
+ struct vmci_handle_arr *array;
+
+ if (capacity == 0)
+ capacity = VMCI_HANDLE_ARRAY_DEFAULT_SIZE;
+
+ array = kmalloc(handle_arr_calc_size(capacity), GFP_ATOMIC);
+ if (!array)
+ return NULL;
+
+ array-capacity = capacity;
+ array-size = 0;
+
+ return array;
+}
+
+void vmci_handle_arr_destroy(struct vmci_handle_arr *array)
+{
+ kfree(array);
+}
+
+void vmci_handle_arr_append_entry(struct vmci_handle_arr **array_ptr,
+ struct vmci_handle handle)
+{
+ struct vmci_handle_arr *array;
+
+ BUG_ON(!array_ptr || !*array_ptr);
+ array = *array_ptr;
+
+ if (unlikely(array-size = array-capacity)) {
+ /* reallocate. */
+ struct vmci_handle_arr *new_array;
+ size_t new_capacity = array-capacity * VMCI_ARR_CAP_MULT;
+ size_t new_size = handle_arr_calc_size(new_capacity);
+
+ new_array = krealloc(array, new_size, GFP_ATOMIC);
+ if (!new_array)
+ return;
+
+ new_array-capacity = new_capacity;
+ *array_ptr = array = new_array;
+ }
+
+ array-entries[array-size] = handle;
+ array-size++;
+}
+
+/*
+ * Handle that was removed, VMCI_INVALID_HANDLE if entry not found.
+ */
+struct vmci_handle vmci_handle_arr_remove_entry(struct vmci_handle_arr *array,
+ struct vmci_handle entry_handle)
+{
+ struct vmci_handle handle = VMCI_INVALID_HANDLE;
+ size_t i;
+
+ BUG_ON(!array);
+
+ for (i = 0; i array-size; i++) {
+ if (VMCI_HANDLE_EQUAL(array-entries[i], entry_handle)) {
+ handle = array-entries[i];
+ array-size--;
+ array-entries[i] = array-entries[array-size];
+ array-entries[array-size] = VMCI_INVALID_HANDLE;
+ break;
+ }
+ }
+
+ return handle;
+}
+
+/*
+ * Handle that was removed, VMCI_INVALID_HANDLE if array was empty.
+ */
+struct vmci_handle vmci_handle_arr_remove_tail(struct vmci_handle_arr *array)
+{
+ struct vmci_handle handle = VMCI_INVALID_HANDLE;
+
+ BUG_ON(!array);
+
+ if (array-size) {
+ array-size--;
+ handle = array-entries[array-size];
+ array-entries[array-size] = VMCI_INVALID_HANDLE;
+ }
+
+ return handle;
+}
+
+/*
+ * Handle at given index, VMCI_INVALID_HANDLE if invalid index.
+ */
+struct vmci_handle
+vmci_handle_arr_get_entry(const struct vmci_handle_arr *array, size_t index)
+{
+ BUG_ON(!array);
+
+ if (unlikely(index = array-size))
+ return VMCI_INVALID_HANDLE;
+
+ return array-entries[index];
+}
+
+size_t vmci_handle_arr_get_size(const struct vmci_handle_arr *array)
+{
+ BUG_ON(!array);
+
+ return array-size;
+}
+
+bool vmci_handle_arr_has_entry(const struct vmci_handle_arr *array,
+ struct vmci_handle entry_handle)
+{
+ size_t i;
+
+ BUG_ON(!array);
+
+ for (i = 0; i array-size; i++)
+ if (VMCI_HANDLE_EQUAL(array-entries[i], entry_handle))
+ return true;
+
+ return false;
+}
+
+/*
+ * NULL if the array is empty. Otherwise, a pointer to the array
+ * of VMCI handles in the handle array.
+ */
+struct vmci_handle
[PATCH 08/12] VMCI: resource object implementation.
VMCI resource tracks all used resources within the vmci code.
Signed-off-by: George Zhang georgezh...@vmware.com
---
drivers/misc/vmw_vmci/vmci_resource.c | 237 +
drivers/misc/vmw_vmci/vmci_resource.h | 59
2 files changed, 296 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/vmw_vmci/vmci_resource.c
create mode 100644 drivers/misc/vmw_vmci/vmci_resource.h
diff --git a/drivers/misc/vmw_vmci/vmci_resource.c
b/drivers/misc/vmw_vmci/vmci_resource.c
new file mode 100644
index 000..a2f5fd0
--- /dev/null
+++ b/drivers/misc/vmw_vmci/vmci_resource.c
@@ -0,0 +1,237 @@
+/*
+ * VMware VMCI Driver
+ *
+ * Copyright (C) 2012 VMware, Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation version 2 and no later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include linux/vmw_vmci_defs.h
+#include linux/hash.h
+#include linux/types.h
+#include linux/rculist.h
+
+#include vmci_common_int.h
+#include vmci_resource.h
+#include vmci_driver.h
+
+
+#define VMCI_RESOURCE_HASH_BITS 7
+#define VMCI_RESOURCE_HASH_BUCKETS (1 VMCI_RESOURCE_HASH_BITS)
+
+struct vmci_hash_table {
+ spinlock_t lock;
+ struct hlist_head entries[VMCI_RESOURCE_HASH_BUCKETS];
+};
+
+static struct vmci_hash_table vmci_resource_table = {
+ .lock = __SPIN_LOCK_UNLOCKED(vmci_resource_table.lock),
+};
+
+static unsigned int vmci_resource_hash(struct vmci_handle handle)
+{
+ return hash_32(VMCI_HANDLE_TO_RESOURCE_ID(handle),
+ VMCI_RESOURCE_HASH_BITS);
+}
+
+/*
+ * Gets a resource (if one exists) matching given handle from the hash table.
+ */
+static struct vmci_resource *vmci_resource_lookup(struct vmci_handle handle)
+{
+ struct vmci_resource *r, *resource = NULL;
+ struct hlist_node *node;
+ unsigned int idx = vmci_resource_hash(handle);
+
+ BUG_ON(VMCI_HANDLE_EQUAL(handle, VMCI_INVALID_HANDLE));
+
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(r, node,
+vmci_resource_table.entries[idx], node) {
+ u32 rid = VMCI_HANDLE_TO_RESOURCE_ID(r-handle);
+ u32 cid = VMCI_HANDLE_TO_CONTEXT_ID(r-handle);
+
+ if (rid == VMCI_HANDLE_TO_RESOURCE_ID(handle)
+ (cid == VMCI_HANDLE_TO_CONTEXT_ID(handle) ||
+cid == VMCI_INVALID_ID)) {
+ resource = r;
+ break;
+ }
+ }
+ rcu_read_unlock();
+
+ return resource;
+}
+
+/*
+ * Find an unused resource ID and return it. The first
+ * VMCI_RESERVED_RESOURCE_ID_MAX are reserved so we start from
+ * its value + 1.
+ * Returns VMCI resource id on success, VMCI_INVALID_ID on failure.
+ */
+static u32 vmci_resource_find_id(u32 context_id)
+{
+ static u32 resource_id = VMCI_RESERVED_RESOURCE_ID_MAX + 1;
+ u32 old_rid = resource_id;
+ u32 current_rid;
+
+ /*
+* Generate a unique resource ID. Keep on trying until we wrap around
+* in the RID space.
+*/
+ BUG_ON(old_rid = VMCI_RESERVED_RESOURCE_ID_MAX);
+
+ do {
+ struct vmci_handle handle;
+
+ current_rid = resource_id;
+ resource_id++;
+ if (unlikely(resource_id == VMCI_INVALID_ID)) {
+ /* Skip the reserved rids. */
+ resource_id = VMCI_RESERVED_RESOURCE_ID_MAX + 1;
+ }
+
+ handle = vmci_make_handle(context_id, current_rid);
+ if (!vmci_resource_lookup(handle))
+ return current_rid;
+ } while (resource_id != old_rid);
+
+ return VMCI_INVALID_ID;
+}
+
+
+int vmci_resource_add(struct vmci_resource *resource,
+ enum vmci_resource_type resource_type,
+ struct vmci_handle handle)
+
+{
+ unsigned int idx;
+ int result;
+
+ BUG_ON(!resource);
+
+ spin_lock(vmci_resource_table.lock);
+
+ if (handle.resource == VMCI_INVALID_ID) {
+ handle.resource = vmci_resource_find_id(handle.context);
+ if (handle.resource == VMCI_INVALID_ID) {
+ result = VMCI_ERROR_NO_HANDLE;
+ goto out;
+ }
+ } else if (vmci_resource_lookup(handle)) {
+ result = VMCI_ERROR_ALREADY_EXISTS;
+ goto out;
+ }
+
+ resource-handle = handle;
+ resource-type = resource_type;
+ INIT_HLIST_NODE(resource-node);
+ kref_init(resource-kref);
+ init_completion(resource-done);
Re: [PATCH 00/12] VMCI for Linux upstreaming
On Mon, Oct 29, 2012 at 06:03:28PM -0700, George Zhang wrote: drivers/misc/Kconfig |1 drivers/misc/Makefile |2 drivers/misc/vmw_vmci/Kconfig | 16 drivers/misc/vmw_vmci/Makefile| 43 Meta comment here, why drivers/misc/? The other hypervisor infrastructures all have their own directory under drivers/ Should we be moving everything to drivers/hyperv/ somehow? thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/12] VMCI: device driver implementaton.
On Mon, Oct 29, 2012 at 06:04:15PM -0700, George Zhang wrote:
+/*
+ * vmci_get_context_id() - Gets the current context ID.
+ *
+ * Returns the current context ID. Note that since this is accessed only
+ * from code running in the host, this always returns the host context ID.
+ */
+u32 vmci_get_context_id(void)
+{
+ if (vmci_guest_code_active()) {
+ return vmci_get_vm_context_id();
+ } else if (vmci_host_code_active()) {
+ return VMCI_HOST_CONTEXT_ID;
+ } else {
+ return VMCI_INVALID_ID;
+ }
+}
+EXPORT_SYMBOL_GPL(vmci_get_context_id);
checkpatch didn't complain about too many { } here?
+/*
+ * vmci_version() - Returns the version of the driver.
+ *
+ * Returns the version of the VMCI driver.
+ */
+u32 vmci_version(void)
+{
+ return VMCI_VERSION;
+}
+EXPORT_SYMBOL_GPL(vmci_version);
Why is this needed? Why would a kernel driver ever care about this?
thanks,
greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/12] VMCI: device driver implementaton.
On Mon, Oct 29, 2012 at 06:04:15PM -0700, George Zhang wrote:
+static int __init vmci_core_init(void)
+{
+ int result;
+
+ result = vmci_ctx_init();
+ if (result VMCI_SUCCESS) {
+ pr_err(Failed to initialize VMCIContext (result=%d).\n,
+ result);
If you are going to use pr_* functions, it's usually a good idea to
define pr_fmt in a consistant way so that it shows up the same across
all of your .c files. See other drivers for examples of how to do this
properly.
+ return -EINVAL;
+ }
+
+ result = vmci_datagram_init();
+ if (result VMCI_SUCCESS) {
+ pr_err(Failed to initialize VMCIDatagram (result=%d).\n,
+ result);
+ return -EINVAL;
+ }
+
+ result = vmci_event_init();
+ if (result VMCI_SUCCESS) {
+ pr_err(Failed to initialize VMCIEvent (result=%d).\n, result);
+ return -EINVAL;
+ }
You don't free and unwind things properly if things go wrong here, why
not?
+
+ return 0;
+}
+
+static void __exit vmci_core_exit(void)
+{
+ vmci_event_exit();
+}
+
+static int __init vmci_drv_init(void)
+{
+ int error;
+
+ error = vmci_core_init();
+ if (error)
+ return error;
+
+ if (!vmci_disable_guest) {
+ error = vmci_guest_init();
+ if (error) {
+ pr_warn(Failed to initialize guest personality
(err=%d).\n,
+ error);
+ } else {
+ vmci_guest_personality_initialized = true;
+ pr_info(Guest personality initialized and is %s\n,
+ vmci_guest_code_active() ?
+ active : inactive);
+ }
+ }
+
+ if (!vmci_disable_host) {
+ error = vmci_host_init();
+ if (error) {
+ pr_warn(Unable to initialize host personality
(err=%d).\n,
+ error);
+ } else {
+ vmci_host_personality_initialized = true;
+ pr_info(Initialized host personality\n);
+ }
+ }
+
+ if (!vmci_guest_personality_initialized
+ !vmci_host_personality_initialized) {
+ vmci_core_exit();
+ return -ENODEV;
+ }
+
+ pr_debug(Module is initialized\n);
Really? You need this message still?
+ return 0;
+}
+module_init(vmci_drv_init);
+
+static void __exit vmci_drv_exit(void)
+{
+ if (vmci_guest_personality_initialized)
+ vmci_guest_exit();
+
+ if (vmci_host_personality_initialized)
+ vmci_host_exit();
+
+ vmci_core_exit();
+ pr_debug(Module is unloaded\n);
Same here, is this really needed?
thanks,
greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 05/12] VMCI: event handling implementation.
On Mon, Oct 29, 2012 at 06:04:27PM -0700, George Zhang wrote:
+/*
+ * Releases the given VMCISubscription.
+ * Fires the destroy event if the reference count has gone to zero.
+ */
+static void event_release(struct vmci_subscription *entry)
+{
+ kref_put(entry-kref, event_signal_destroy);
+}
Same question as before with the kref_put() call, what is handling the
locking here? It looks like a race to me.
thanks,
greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 05/12] VMCI: event handling implementation.
On Mon, Oct 29, 2012 at 06:04:27PM -0700, George Zhang wrote:
+static void event_signal_destroy(struct kref *kref)
+{
+ struct vmci_subscription *entry =
+ container_of(kref, struct vmci_subscription, kref);
+
+ complete(entry-done);
+}
Didn't you just leak memory here? What frees the structure up?
thanks,
greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 08/12] VMCI: resource object implementation.
On Mon, Oct 29, 2012 at 06:04:58PM -0700, George Zhang wrote:
+static struct vmci_resource *vmci_resource_lookup(struct vmci_handle handle)
+{
+ struct vmci_resource *r, *resource = NULL;
+ struct hlist_node *node;
+ unsigned int idx = vmci_resource_hash(handle);
+
+ BUG_ON(VMCI_HANDLE_EQUAL(handle, VMCI_INVALID_HANDLE));
You just crashed a machine, with no chance for recovery. Not a good
idea. Never a good idea. Customers just lost data, and now they are
mad. Make sure you at least print out your email address so they know
who to blame :)
Seriously, never BUG() in a driver, warn, sure, but this just looks like
a debugging assert(). Please remove all of these, they are sprinkled
all over the driver code here, I'm only responding to one of them here.
Even better yet, properly handle the error and keep on going, that's
what the rest of the kernel does. Or should :)
thanks,
greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 08/12] VMCI: resource object implementation.
On Mon, Oct 29, 2012 at 06:04:58PM -0700, George Zhang wrote: VMCI resource tracks all used resources within the vmci code. Same kref_put() with no lock seen question in this file, prove me wrong please. thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Pv-drivers] [PATCH 05/12] VMCI: event handling implementation.
On Mon, Oct 29, 2012 at 07:26:05PM -0700, Greg KH wrote:
On Mon, Oct 29, 2012 at 06:04:27PM -0700, George Zhang wrote:
+static void event_signal_destroy(struct kref *kref)
+{
+ struct vmci_subscription *entry =
+ container_of(kref, struct vmci_subscription, kref);
+
+ complete(entry-done);
+}
Didn't you just leak memory here? What frees the structure up?
event_unregister_subscription() waits for that completion and frees the
structure. We want event_unregister_subscription() to wait until all
fired callbacks completed before unregister is complete.
Thanks,
Dmitry
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Pv-drivers] [PATCH 08/12] VMCI: resource object implementation.
On Mon, Oct 29, 2012 at 07:29:46PM -0700, Greg KH wrote: On Mon, Oct 29, 2012 at 06:04:58PM -0700, George Zhang wrote: VMCI resource tracks all used resources within the vmci code. Same kref_put() with no lock seen question in this file, prove me wrong please. Same proof as with others, the reference can't be taken unless the resource is in hast table (which protected by RCU/spinlock) so no fear of bouncing off 0. Thanks, Dmitry ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Pv-drivers] [PATCH 12/12] VMCI: Some header and config files.
On Mon, Oct 29, 2012 at 07:32:55PM -0700, Greg KH wrote: On Mon, Oct 29, 2012 at 06:05:38PM -0700, George Zhang wrote: --- /dev/null +++ b/drivers/misc/vmw_vmci/Makefile @@ -0,0 +1,43 @@ + +# +# Linux driver for VMware's VMCI device. +# +# Copyright (C) 2007-2012, VMware, Inc. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; version 2 of the License and no later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or +# NON INFRINGEMENT. See the GNU General Public License for more +# details. +# +# Maintained by: Andrew Stiegmann pv-driv...@vmware.com +# + That's a big boiler-plate for a makefile :) Wait, what's Andrew's name doing here, and yet it isn't on any of the signed-off-by: or From: lines of the driver? Surely you aren't the only contributor here? +# +# Makefile for the VMware VMCI +# That's the only needed comment for this file, if even that. + +obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci.o + +vmw_vmci-y += vmci_context.o +vmw_vmci-y += vmci_datagram.o +vmw_vmci-y += vmci_doorbell.o +vmw_vmci-y += vmci_driver.o +vmw_vmci-y += vmci_event.o +vmw_vmci-y += vmci_guest.o +vmw_vmci-y += vmci_handle_array.o +vmw_vmci-y += vmci_host.o +vmw_vmci-y += vmci_queue_pair.o +vmw_vmci-y += vmci_resource.o +vmw_vmci-y += vmci_route.o You can do this cleaner with multiple .o objects on the same line... +vmci: + $(MAKE) -C ../../.. SUBDIRS=$$PWD CONFIG_VMWARE_VMCI=m modules + +clean: + $(MAKE) -C ../../.. SUBDIRS=$$PWD CONFIG_VMWARE_VMCI=m clean What are these two last targets for? I'm guessing this is from when you ported it from a stand-along module? Please remove them. We'll clean it all up. Thanks for going over the code. -- Dmitry ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
