From 39e4f9388f19c1eaae092590d2e2f475ac15c062 Mon Sep 17 00:00:00 2001
From: Sreenath Sharma <sreenath.sharma@broadcom.com>
Date: Tue, 31 May 2016 19:14:32 +0530
Subject: [PATCH] net: wireless: bcmdhd: fix enqueue and dequeue of deferred
work framework
- increase priority work fifo size to 16
- align deferred work data to the power of two
- check for enough room before enqueue to avoid incomplete data
- check for enough data before dequeue to avoid incomplete data
- proper handling of enqueue and dequeue return values
BUG=28226991
Change-Id: I3be11134bb37bcb99157ff2f58cd7f775cd11260
Signed-off-by: Sreenath Sharma <sreenath.sharma@broadcom.com>
---
drivers/net/wireless/bcmdhd/dhd_linux.c | 14 +-
drivers/net/wireless/bcmdhd/dhd_linux_wq.c | 218 ++++++++++++++-------
drivers/net/wireless/bcmdhd/dhd_linux_wq.h | 8 +-
3 files changed, 157 insertions(+), 83 deletions(-)
diff --git a/drivers/net/wireless/bcmdhd/dhd_linux.c b/drivers/net/wireless/bcmdhd/dhd_linux.c
index 8a6293cea150..270078b21046 100644
--- a/drivers/net/wireless/bcmdhd/dhd_linux.c
+++ b/drivers/net/wireless/bcmdhd/dhd_linux.c
@@ -2331,7 +2331,7 @@ dhd_set_mac_address(struct net_device *dev, void *addr)
dhdif->set_macaddress = TRUE;
dhd_net_if_unlock_local(dhd);
dhd_deferred_schedule_work(dhd->dhd_deferred_wq, (void *)dhdif, DHD_WQ_WORK_SET_MAC,
- dhd_set_mac_addr_handler, DHD_WORK_PRIORITY_LOW);
+ dhd_set_mac_addr_handler, DHD_WQ_WORK_PRIORITY_LOW);
return ret;
}
@@ -2347,7 +2347,7 @@ dhd_set_multicast_list(struct net_device *dev)
dhd->iflist[ifidx]->set_multicast = TRUE;
dhd_deferred_schedule_work(dhd->dhd_deferred_wq, (void *)dhd->iflist[ifidx],
- DHD_WQ_WORK_SET_MCAST_LIST, dhd_set_mcast_list_handler, DHD_WORK_PRIORITY_LOW);
+ DHD_WQ_WORK_SET_MCAST_LIST, dhd_set_mcast_list_handler, DHD_WQ_WORK_PRIORITY_LOW);
}
#ifdef PROP_TXSTATUS
@@ -4468,7 +4468,7 @@ dhd_event_ifadd(dhd_info_t *dhdinfo, wl_event_data_if_t *ifevent, char *name, ui
strncpy(if_event->name, name, IFNAMSIZ);
if_event->name[IFNAMSIZ - 1] = '\0';
dhd_deferred_schedule_work(dhdinfo->dhd_deferred_wq, (void *)if_event,
- DHD_WQ_WORK_IF_ADD, dhd_ifadd_event_handler, DHD_WORK_PRIORITY_LOW);
+ DHD_WQ_WORK_IF_ADD, dhd_ifadd_event_handler, DHD_WQ_WORK_PRIORITY_LOW);
}
return BCME_OK;
@@ -4495,7 +4495,7 @@ dhd_event_ifdel(dhd_info_t *dhdinfo, wl_event_data_if_t *ifevent, char *name, ui
strncpy(if_event->name, name, IFNAMSIZ);
if_event->name[IFNAMSIZ - 1] = '\0';
dhd_deferred_schedule_work(dhdinfo->dhd_deferred_wq, (void *)if_event, DHD_WQ_WORK_IF_DEL,
- dhd_ifdel_event_handler, DHD_WORK_PRIORITY_LOW);
+ dhd_ifdel_event_handler, DHD_WQ_WORK_PRIORITY_LOW);
return BCME_OK;
}
@@ -6736,7 +6736,7 @@ dhd_inet6addr_notifier_call(struct notifier_block *this, unsigned long event, vo
/* defer the work to thread as it may block kernel */
dhd_deferred_schedule_work(dhd->dhd_deferred_wq, (void *)ndo_info, DHD_WQ_WORK_IPV6_NDO,
- dhd_inet6_work_handler, DHD_WORK_PRIORITY_LOW);
+ dhd_inet6_work_handler, DHD_WQ_WORK_PRIORITY_LOW);
return NOTIFY_DONE;
}
#endif /* #ifdef CONFIG_IPV6 */
@@ -9348,7 +9348,7 @@ int dhd_os_send_hang_message(dhd_pub_t *dhdp)
if (!dhdp->hang_was_sent) {
dhdp->hang_was_sent = 1;
dhd_deferred_schedule_work(dhdp->info->dhd_deferred_wq, (void *)dhdp,
- DHD_WQ_WORK_HANG_MSG, dhd_hang_process, DHD_WORK_PRIORITY_HIGH);
+ DHD_WQ_WORK_HANG_MSG, dhd_hang_process, DHD_WQ_WORK_PRIORITY_HIGH);
}
}
return ret;
@@ -10660,7 +10660,7 @@ void dhd_schedule_memdump(dhd_pub_t *dhdp, uint8 *buf, uint32 size)
dump->bufsize = size;
dhd_deferred_schedule_work(dhdp->info->dhd_deferred_wq, (void *)dump,
- DHD_WQ_WORK_SOC_RAM_DUMP, dhd_mem_dump_to_file, DHD_WORK_PRIORITY_HIGH);
+ DHD_WQ_WORK_SOC_RAM_DUMP, dhd_mem_dump_to_file, DHD_WQ_WORK_PRIORITY_HIGH);
}
int dhd_os_socram_dump(struct net_device *dev, uint32 *dump_size)
{
diff --git a/drivers/net/wireless/bcmdhd/dhd_linux_wq.c b/drivers/net/wireless/bcmdhd/dhd_linux_wq.c
index 2d01570dd2e5..fe3ccd5e30d1 100644
--- a/drivers/net/wireless/bcmdhd/dhd_linux_wq.c
+++ b/drivers/net/wireless/bcmdhd/dhd_linux_wq.c
@@ -3,13 +3,13 @@
* Generic interface to handle dhd deferred work events
*
* Copyright (C) 1999-2014, Broadcom Corporation
- *
+ *
* Unless you and Broadcom execute a separate written software license
* agreement governing use of this software, this software is licensed to you
* under the terms of the GNU General Public License version 2 (the "GPL"),
* available at http://www.broadcom.com/licenses/GPLv2.php, with the
* following added to such license:
- *
+ *
* As a special exception, the copyright holders of this software give you
* permission to link this software with independent modules, and to copy and
* distribute the resulting executable under terms of your choice, provided that
@@ -17,7 +17,7 @@
* the license of that module. An independent module is a module which is not
* derived from this software. The special exception does not apply to any
* modifications of the software.
- *
+ *
* Notwithstanding the above, under no circumstances may you combine this
* software in any way with any other Broadcom software provided under a license
* other than the GPL, without Broadcom's express prior written consent.
@@ -43,35 +43,48 @@
#include <dhd_dbg.h>
#include <dhd_linux_wq.h>
-struct dhd_deferred_event_t {
- u8 event; /* holds the event */
- void *event_data; /* Holds event specific data */
+/*
+ * XXX: always make sure that the size of this structure is aligned to
+ * the power of 2 (2^n) i.e, if any new variable has to be added then
+ * modify the padding accordingly
+ */
+typedef struct dhd_deferred_event {
+ u8 event; /* holds the event */
+ void *event_data; /* holds event specific data */
event_handler_t event_handler;
-};
-#define DEFRD_EVT_SIZE sizeof(struct dhd_deferred_event_t)
+ unsigned long pad; /* for memory alignment to power of 2 */
+} dhd_deferred_event_t;
-struct dhd_deferred_wq {
- struct work_struct deferred_work; /* should be the first member */
+#define DEFRD_EVT_SIZE (sizeof(dhd_deferred_event_t))
- /*
- * work events may occur simultaneously.
- * Can hold upto 64 low priority events and 4 high priority events
- */
-#define DHD_PRIO_WORK_FIFO_SIZE (4 * sizeof(struct dhd_deferred_event_t))
-#define DHD_WORK_FIFO_SIZE (64 * sizeof(struct dhd_deferred_event_t))
- struct kfifo *prio_fifo;
- struct kfifo *work_fifo;
- u8 *prio_fifo_buf;
- u8 *work_fifo_buf;
- spinlock_t work_lock;
- void *dhd_info; /* review: does it require */
+/*
+ * work events may occur simultaneously.
+ * can hold upto 64 low priority events and 16 high priority events
+ */
+#define DHD_PRIO_WORK_FIFO_SIZE (16 * DEFRD_EVT_SIZE)
+#define DHD_WORK_FIFO_SIZE (64 * DEFRD_EVT_SIZE)
+
+#define DHD_FIFO_HAS_FREE_SPACE(fifo) \
+ ((fifo) && (kfifo_avail(fifo) >= DEFRD_EVT_SIZE))
+#define DHD_FIFO_HAS_ENOUGH_DATA(fifo) \
+ ((fifo) && (kfifo_len(fifo) >= DEFRD_EVT_SIZE))
+
+struct dhd_deferred_wq {
+ struct work_struct deferred_work; /* should be the first member */
+
+ struct kfifo *prio_fifo;
+ struct kfifo *work_fifo;
+ u8 *prio_fifo_buf;
+ u8 *work_fifo_buf;
+ spinlock_t work_lock;
+ void *dhd_info; /* review: does it require */
};
static inline struct kfifo*
dhd_kfifo_init(u8 *buf, int size, spinlock_t *lock)
{
struct kfifo *fifo;
- gfp_t flags = CAN_SLEEP()? GFP_KERNEL : GFP_ATOMIC;
+ gfp_t flags = CAN_SLEEP() ? GFP_KERNEL : GFP_ATOMIC;
#if (LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 33))
fifo = kfifo_init(buf, size, flags, lock);
@@ -101,10 +114,10 @@ static void dhd_deferred_work_handler(struct work_struct *data);
void*
dhd_deferred_work_init(void *dhd_info)
{
- struct dhd_deferred_wq *work = NULL;
- u8* buf;
- unsigned long fifo_size = 0;
- gfp_t flags = CAN_SLEEP()? GFP_KERNEL : GFP_ATOMIC;
+ struct dhd_deferred_wq *work = NULL;
+ u8* buf;
+ unsigned long fifo_size = 0;
+ gfp_t flags = CAN_SLEEP() ? GFP_KERNEL : GFP_ATOMIC;
if (!dhd_info) {
DHD_ERROR(("%s: dhd info not initialized\n", __FUNCTION__));
@@ -113,9 +126,8 @@ dhd_deferred_work_init(void *dhd_info)
work = (struct dhd_deferred_wq *)kzalloc(sizeof(struct dhd_deferred_wq),
flags);
-
if (!work) {
- DHD_ERROR(("%s: work queue creation failed \n", __FUNCTION__));
+ DHD_ERROR(("%s: work queue creation failed\n", __FUNCTION__));
goto return_null;
}
@@ -126,10 +138,12 @@ dhd_deferred_work_init(void *dhd_info)
/* allocate buffer to hold prio events */
fifo_size = DHD_PRIO_WORK_FIFO_SIZE;
- fifo_size = is_power_of_2(fifo_size)? fifo_size : roundup_pow_of_two(fifo_size);
+ fifo_size = is_power_of_2(fifo_size) ? fifo_size :
+ roundup_pow_of_two(fifo_size);
buf = (u8*)kzalloc(fifo_size, flags);
if (!buf) {
- DHD_ERROR(("%s: prio work fifo allocation failed \n", __FUNCTION__));
+ DHD_ERROR(("%s: prio work fifo allocation failed\n",
+ __FUNCTION__));
goto return_null;
}
@@ -142,10 +156,11 @@ dhd_deferred_work_init(void *dhd_info)
/* allocate buffer to hold work events */
fifo_size = DHD_WORK_FIFO_SIZE;
- fifo_size = is_power_of_2(fifo_size)? fifo_size : roundup_pow_of_two(fifo_size);
+ fifo_size = is_power_of_2(fifo_size) ? fifo_size :
+ roundup_pow_of_two(fifo_size);
buf = (u8*)kzalloc(fifo_size, flags);
if (!buf) {
- DHD_ERROR(("%s: work fifo allocation failed \n", __FUNCTION__));
+ DHD_ERROR(("%s: work fifo allocation failed\n", __FUNCTION__));
goto return_null;
}
@@ -157,13 +172,13 @@ dhd_deferred_work_init(void *dhd_info)
}
work->dhd_info = dhd_info;
- DHD_ERROR(("%s: work queue initialized \n", __FUNCTION__));
+ DHD_ERROR(("%s: work queue initialized\n", __FUNCTION__));
return work;
return_null:
-
- if (work)
+ if (work) {
dhd_deferred_work_deinit(work);
+ }
return NULL;
}
@@ -175,7 +190,8 @@ dhd_deferred_work_deinit(void *work)
if (!deferred_work) {
- DHD_ERROR(("%s: deferred work has been freed alread \n", __FUNCTION__));
+ DHD_ERROR(("%s: deferred work has been freed already\n",
+ __FUNCTION__));
return;
}
@@ -186,15 +202,31 @@ dhd_deferred_work_deinit(void *work)
* free work event fifo.
* kfifo_free frees locally allocated fifo buffer
*/
- if (deferred_work->prio_fifo)
+ if (deferred_work->prio_fifo) {
dhd_kfifo_free(deferred_work->prio_fifo);
+ }
- if (deferred_work->work_fifo)
+ if (deferred_work->work_fifo) {
dhd_kfifo_free(deferred_work->work_fifo);
+ }
kfree(deferred_work);
}
+/* select kfifo according to priority */
+static inline struct kfifo *
+dhd_deferred_work_select_kfifo(struct dhd_deferred_wq *deferred_wq,
+ u8 priority)
+{
+ if (priority == DHD_WQ_WORK_PRIORITY_HIGH) {
+ return deferred_wq->prio_fifo;
+ } else if (priority == DHD_WQ_WORK_PRIORITY_LOW) {
+ return deferred_wq->work_fifo;
+ } else {
+ return NULL;
+ }
+}
+
/*
* Prepares event to be queued
* Schedules the event
@@ -203,21 +235,29 @@ int
dhd_deferred_schedule_work(void *workq, void *event_data, u8 event,
event_handler_t event_handler, u8 priority)
{
- struct dhd_deferred_wq *deferred_wq = (struct dhd_deferred_wq *) workq;
- struct dhd_deferred_event_t deferred_event;
- int status;
+ struct dhd_deferred_wq *deferred_wq = (struct dhd_deferred_wq *)workq;
+ struct kfifo *fifo;
+ dhd_deferred_event_t deferred_event;
+ int bytes_copied = 0;
if (!deferred_wq) {
- DHD_ERROR(("%s: work queue not initialized \n", __FUNCTION__));
+ DHD_ERROR(("%s: work queue not initialized\n", __FUNCTION__));
ASSERT(0);
return DHD_WQ_STS_UNINITIALIZED;
}
if (!event || (event >= DHD_MAX_WQ_EVENTS)) {
- DHD_ERROR(("%s: Unknown event \n", __FUNCTION__));
+ DHD_ERROR(("%s: unknown event, event=%d\n", __FUNCTION__,
+ event));
return DHD_WQ_STS_UNKNOWN_EVENT;
}
+ if (!priority || (priority >= DHD_WQ_MAX_PRIORITY)) {
+ DHD_ERROR(("%s: unknown priority, priority=%d\n",
+ __FUNCTION__, priority));
+ return DHD_WQ_STS_UNKNOWN_PRIORITY;
+ }
+
/*
* default element size is 1, which can be changed
* using kfifo_esize(). Older kernel(FC11) doesn't support
@@ -231,28 +271,29 @@ dhd_deferred_schedule_work(void *workq, void *event_data, u8 event,
deferred_event.event_data = event_data;
deferred_event.event_handler = event_handler;
- if (priority == DHD_WORK_PRIORITY_HIGH) {
- status = kfifo_in_spinlocked(deferred_wq->prio_fifo, &deferred_event,
- DEFRD_EVT_SIZE, &deferred_wq->work_lock);
- } else {
- status = kfifo_in_spinlocked(deferred_wq->work_fifo, &deferred_event,
+ fifo = dhd_deferred_work_select_kfifo(deferred_wq, priority);
+ if (DHD_FIFO_HAS_FREE_SPACE(fifo)) {
+ bytes_copied = kfifo_in_spinlocked(fifo, &deferred_event,
DEFRD_EVT_SIZE, &deferred_wq->work_lock);
}
-
- if (!status) {
+ if (bytes_copied != DEFRD_EVT_SIZE) {
+ DHD_ERROR(("%s: failed to schedule deferred work, "
+ "priority=%d, bytes_copied=%d\n", __FUNCTION__,
+ priority, bytes_copied));
return DHD_WQ_STS_SCHED_FAILED;
}
schedule_work((struct work_struct *)deferred_wq);
return DHD_WQ_STS_OK;
}
-static int
-dhd_get_scheduled_work(struct dhd_deferred_wq *deferred_wq, struct dhd_deferred_event_t *event)
+static bool
+dhd_get_scheduled_work(struct dhd_deferred_wq *deferred_wq,
+ dhd_deferred_event_t *event)
{
- int status = 0;
+ int bytes_copied = 0;
if (!deferred_wq) {
- DHD_ERROR(("%s: work queue not initialized \n", __FUNCTION__));
+ DHD_ERROR(("%s: work queue not initialized\n", __FUNCTION__));
return DHD_WQ_STS_UNINITIALIZED;
}
@@ -265,17 +306,36 @@ dhd_get_scheduled_work(struct dhd_deferred_wq *deferred_wq, struct dhd_deferred_
ASSERT(kfifo_esize(deferred_wq->prio_fifo) == 1);
ASSERT(kfifo_esize(deferred_wq->work_fifo) == 1);
- /* first read priorit event fifo */
- status = kfifo_out_spinlocked(deferred_wq->prio_fifo, event,
- DEFRD_EVT_SIZE, &deferred_wq->work_lock);
+ /* handle priority work */
+ if (DHD_FIFO_HAS_ENOUGH_DATA(deferred_wq->prio_fifo)) {
+ bytes_copied = kfifo_out_spinlocked(deferred_wq->prio_fifo,
+ event, DEFRD_EVT_SIZE, &deferred_wq->work_lock);
+ }
- if (!status) {
- /* priority fifo is empty. Now read low prio work fifo */
- status = kfifo_out_spinlocked(deferred_wq->work_fifo, event,
- DEFRD_EVT_SIZE, &deferred_wq->work_lock);
+ /* handle normal work if priority work doesn't have enough data */
+ if ((bytes_copied != DEFRD_EVT_SIZE) &&
+ DHD_FIFO_HAS_ENOUGH_DATA(deferred_wq->work_fifo)) {
+ bytes_copied = kfifo_out_spinlocked(deferred_wq->work_fifo,
+ event, DEFRD_EVT_SIZE, &deferred_wq->work_lock);
}
- return status;
+ return (bytes_copied == DEFRD_EVT_SIZE);
+}
+
+static inline void
+dhd_deferred_dump_work_event(dhd_deferred_event_t *work_event)
+{
+ if (!work_event) {
+ DHD_ERROR(("%s: work_event is null\n", __FUNCTION__));
+ return;
+ }
+
+ DHD_ERROR(("%s: work_event->event = %d\n", __FUNCTION__,
+ work_event->event));
+ DHD_ERROR(("%s: work_event->event_data = %p\n", __FUNCTION__,
+ work_event->event_data));
+ DHD_ERROR(("%s: work_event->event_handler = %p\n", __FUNCTION__,
+ work_event->event_handler));
}
/*
@@ -284,9 +344,8 @@ dhd_get_scheduled_work(struct dhd_deferred_wq *deferred_wq, struct dhd_deferred_
static void
dhd_deferred_work_handler(struct work_struct *work)
{
- struct dhd_deferred_wq *deferred_work = (struct dhd_deferred_wq *)work;
- struct dhd_deferred_event_t work_event;
- int status;
+ struct dhd_deferred_wq *deferred_work = (struct dhd_deferred_wq *)work;
+ dhd_deferred_event_t work_event;
if (!deferred_work) {
DHD_ERROR(("%s: work queue not initialized\n", __FUNCTION__));
@@ -294,24 +353,35 @@ dhd_deferred_work_handler(struct work_struct *work)
}
do {
- status = dhd_get_scheduled_work(deferred_work, &work_event);
- DHD_TRACE(("%s: event to handle %d \n", __FUNCTION__, status));
- if (!status) {
- DHD_TRACE(("%s: No event to handle %d \n", __FUNCTION__, status));
+ if (!dhd_get_scheduled_work(deferred_work, &work_event)) {
+ DHD_TRACE(("%s: no event to handle\n", __FUNCTION__));
break;
}
- if (work_event.event > DHD_MAX_WQ_EVENTS) {
- DHD_TRACE(("%s: Unknown event %d \n", __FUNCTION__, work_event.event));
- break;
+ if (work_event.event >= DHD_MAX_WQ_EVENTS) {
+ DHD_ERROR(("%s: unknown event\n", __FUNCTION__));
+ dhd_deferred_dump_work_event(&work_event);
+ ASSERT(work_event.event < DHD_MAX_WQ_EVENTS);
+ continue;
+ }
+
+ if (!work_event.event_data) {
+ DHD_ERROR(("%s: event data is null\n", __FUNCTION__));
+ dhd_deferred_dump_work_event(&work_event);
+ ASSERT(work_event.event_data != NULL);
+ continue;
}
if (work_event.event_handler) {
work_event.event_handler(deferred_work->dhd_info,
work_event.event_data, work_event.event);
} else {
- DHD_ERROR(("%s: event not defined %d\n", __FUNCTION__, work_event.event));
+ DHD_ERROR(("%s: event handler is null\n",
+ __FUNCTION__));
+ dhd_deferred_dump_work_event(&work_event);
+ ASSERT(work_event.event_handler != NULL);
}
} while (1);
+
return;
}
diff --git a/drivers/net/wireless/bcmdhd/dhd_linux_wq.h b/drivers/net/wireless/bcmdhd/dhd_linux_wq.h
index 7c37173db8bb..a8cdf4c4f901 100644
--- a/drivers/net/wireless/bcmdhd/dhd_linux_wq.h
+++ b/drivers/net/wireless/bcmdhd/dhd_linux_wq.h
@@ -43,8 +43,11 @@ enum _wq_event {
/*
* Work event priority
*/
-#define DHD_WORK_PRIORITY_LOW 0
-#define DHD_WORK_PRIORITY_HIGH 1
+enum wq_priority {
+ DHD_WQ_WORK_PRIORITY_LOW = 1,
+ DHD_WQ_WORK_PRIORITY_HIGH,
+ DHD_WQ_MAX_PRIORITY
+};
/*
* Error definitions
@@ -54,6 +57,7 @@ enum _wq_event {
#define DHD_WQ_STS_UNINITIALIZED -2
#define DHD_WQ_STS_SCHED_FAILED -3
#define DHD_WQ_STS_UNKNOWN_EVENT -4
+#define DHD_WQ_STS_UNKNOWN_PRIORITY -5
typedef void (*event_handler_t)(void *handle, void *event_data, u8 event);
--
GitLab