[PATCH v2 4/5] staging: greybus: loopback: convert loopback to use generic async operations

Bryan O'Donoghue pure.logic at nexus-software.ie
Tue Dec 27 13:01:38 UTC 2016


Loopback has its own internal method for tracking and timing out
asynchronous operations however previous patches make it possible to use
functionality provided by operation.c to do this instead. Using the code in
operation.c means we can completely subtract the timer, the work-queue, the
kref and the cringe-worthy 'pending' flag. The completion callback
triggered by operation.c will provide an authoritative result code -
including -ETIMEDOUT for asynchronous operations. Tested to ensure the
existing stats given by loopback for synchronous, asynchronous and
asynchronous+timeout still hold.

Before:
gb_loopback_test -i 1000 -s 128 -t sink -p
1970-1-1 7:30:8
 test:			sink
 path:			gb_loopback0
 size:			128
 iterations:		1000
 errors:		0
 async:			Disabled
 requests per-sec:	min=432, max=436, average=434.662994, jitter=4
 ap-throughput B/s:	min=67535 max=68036 average=67807.421875 jitter=501
 ap-latency usec:	min=1626 max=3559 average=2287.077881 jitter=1933
 apbridge-latency usec:	min=0 max=0 average=0.000000 jitter=0
 gbphy-latency usec:	min=0 max=0 average=0.000000 jitter=0

After:
gb_loopback_test -i 1000 -s 128 -t sink -p

1970-1-1 0:49:59
 test:			sink
 path:			gb_loopback0
 size:			128
 iterations:		1000
 errors:		0
 async:			Disabled
 requests per-sec:	min=437, max=442, average=440.065491, jitter=5
 ap-throughput B/s:	min=68212 max=69107 average=68650.218750 jitter=895
 ap-latency usec:	min=1600 max=9472 average=2259.075928 jitter=7872
 apbridge-latency usec:	min=0 max=0 average=0.000000 jitter=0
 gbphy-latency usec:	min=0 max=0 average=0.000000 jitter=0

Before:
gb_loopback_test -i 1000 -s 128 -t sink -x -c 4 -p

arche:/ # gb_loopback_test -i 1000 -s 128 -t sink -x -c 4 -p

1970-1-1 7:31:17
 test:			sink
 path:			gb_loopback0
 size:			128
 iterations:		1000
 errors:		0
 async:			Enabled
 requests per-sec:	min=1513, max=1513, average=1513.713501, jitter=0
 ap-throughput B/s:	min=236139 max=236139 average=236139.296875
jitter=0
 ap-latency usec:	min=1413 max=7656 average=2491.353027 jitter=6243
 apbridge-latency usec:	min=0 max=0 average=0.000000 jitter=0
 gbphy-latency usec:	min=0 max=0 average=0.000000 jitter=0

After:
gb_loopback_test -i 1000 -s 128 -t sink -x -c 4 -p

1970-1-1 0:50:42
 test:			sink
 path:			gb_loopback0
 size:			128
 iterations:		1000
 errors:		0
 async:			Enabled
 requests per-sec:	min=1513, max=1513, average=1513.702026, jitter=0
 ap-throughput B/s:	min=236137 max=236137 average=236137.515625
jitter=0
 ap-latency usec:	min=1584 max=3813 average=2488.056885 jitter=2229
 apbridge-latency usec:	min=0 max=0 average=0.000000 jitter=0
 gbphy-latency usec:	min=0 max=0 average=0.000000 jitter=0

Before:
gb_loopback_test -i 1000 -s 128 -t sink -x -c 4 -p -o 2000

1970-1-1 8:17:46
 test:			sink
 path:			gb_loopback0
 size:			128
 iterations:		1000
 errors:		730
 async:			Enabled
 requests per-sec:	min=353, max=353, average=353.882080, jitter=0
 ap-throughput B/s:	min=55205 max=55205 average=55205.605469 jitter=0
 ap-latency usec:	min=1498 max=14373 average=2574.329590 jitter=12875
 apbridge-latency usec:	min=0 max=0 average=0.000000 jitter=0
 gbphy-latency usec:	min=0 max=0 average=0.000000 jitter=0

After:
gb_loopback_test -i 1000 -s 128 -t sink -x -c 4 -p -o 2000

1970-1-1 0:51:45
 test:			sink
 path:			gb_loopback0
 size:			128
 iterations:		1000
 errors:		645
 async:			Enabled
 requests per-sec:	min=515, max=515, average=515.907410, jitter=0
 ap-throughput B/s:	min=80481 max=80481 average=80481.554688 jitter=0
 ap-latency usec:	min=1442 max=3607 average=2449.278809 jitter=2165
 apbridge-latency usec:	min=0 max=0 average=0.000000 jitter=0
 gbphy-latency usec:	min=0 max=0 average=0.000000 jitter=0

Signed-off-by: Bryan O'Donoghue <pure.logic at nexus-software.ie>
Signed-off-by: Mitchell Tasman <tasman at leaflabs.com>
---
 drivers/staging/greybus/loopback.c | 167 +++++++------------------------------
 1 file changed, 32 insertions(+), 135 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 3184dd3..5cdd8d0 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -59,11 +59,6 @@ struct gb_loopback_async_operation {
 	struct gb_loopback *gb;
 	struct gb_operation *operation;
 	struct timeval ts;
-	struct timer_list timer;
-	struct list_head entry;
-	struct work_struct work;
-	struct kref kref;
-	bool pending;
 	int (*completion)(struct gb_loopback_async_operation *op_async);
 };
 
@@ -445,56 +440,6 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
 	return ret;
 }
 
-static void __gb_loopback_async_operation_destroy(struct kref *kref)
-{
-	struct gb_loopback_async_operation *op_async;
-
-	op_async = container_of(kref, struct gb_loopback_async_operation, kref);
-
-	list_del(&op_async->entry);
-	if (op_async->operation)
-		gb_operation_put(op_async->operation);
-	atomic_dec(&op_async->gb->outstanding_operations);
-	wake_up(&op_async->gb->wq_completion);
-	kfree(op_async);
-}
-
-static void gb_loopback_async_operation_get(struct gb_loopback_async_operation
-					    *op_async)
-{
-	kref_get(&op_async->kref);
-}
-
-static void gb_loopback_async_operation_put(struct gb_loopback_async_operation
-					    *op_async)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&gb_dev.lock, flags);
-	kref_put(&op_async->kref, __gb_loopback_async_operation_destroy);
-	spin_unlock_irqrestore(&gb_dev.lock, flags);
-}
-
-static struct gb_loopback_async_operation *
-	gb_loopback_operation_find(u16 id)
-{
-	struct gb_loopback_async_operation *op_async;
-	bool found = false;
-	unsigned long flags;
-
-	spin_lock_irqsave(&gb_dev.lock, flags);
-	list_for_each_entry(op_async, &gb_dev.list_op_async, entry) {
-		if (op_async->operation->id == id) {
-			gb_loopback_async_operation_get(op_async);
-			found = true;
-			break;
-		}
-	}
-	spin_unlock_irqrestore(&gb_dev.lock, flags);
-
-	return found ? op_async : NULL;
-}
-
 static void gb_loopback_async_wait_all(struct gb_loopback *gb)
 {
 	wait_event(gb->wq_completion,
@@ -506,86 +451,43 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
 	struct gb_loopback_async_operation *op_async;
 	struct gb_loopback *gb;
 	struct timeval te;
-	bool err = false;
+	int result;
 
 	do_gettimeofday(&te);
-	op_async = gb_loopback_operation_find(operation->id);
-	if (!op_async)
-		return;
-
+	result = gb_operation_result(operation);
+	op_async = gb_operation_get_data(operation);
 	gb = op_async->gb;
+
 	mutex_lock(&gb->mutex);
 
-	if (!op_async->pending || gb_operation_result(operation)) {
-		err = true;
-	} else {
-		if (op_async->completion)
-			if (op_async->completion(op_async))
-				err = true;
-	}
+	if (!result && op_async->completion)
+		result = op_async->completion(op_async);
 
-	if (!err) {
+	if (!result) {
 		gb_loopback_push_latency_ts(gb, &op_async->ts, &te);
 		gb->elapsed_nsecs = gb_loopback_calc_latency(&op_async->ts,
 							     &te);
+	} else {
+		gb->error++;
+		if (result == -ETIMEDOUT)
+			gb->requests_timedout++;
 	}
 
-	if (op_async->pending) {
-		if (err)
-			gb->error++;
-		gb->iteration_count++;
-		op_async->pending = false;
-		del_timer_sync(&op_async->timer);
-		gb_loopback_async_operation_put(op_async);
-		gb_loopback_calculate_stats(gb, err);
-	}
-	mutex_unlock(&gb->mutex);
-
-	dev_dbg(&gb->connection->bundle->dev, "complete operation %d\n",
-		operation->id);
-
-	gb_loopback_async_operation_put(op_async);
-}
-
-static void gb_loopback_async_operation_work(struct work_struct *work)
-{
-	struct gb_loopback *gb;
-	struct gb_operation *operation;
-	struct gb_loopback_async_operation *op_async;
-
-	op_async = container_of(work, struct gb_loopback_async_operation, work);
-	gb = op_async->gb;
-	operation = op_async->operation;
+	gb->iteration_count++;
+	gb_loopback_calculate_stats(gb, result);
 
-	mutex_lock(&gb->mutex);
-	if (op_async->pending) {
-		gb->requests_timedout++;
-		gb->error++;
-		gb->iteration_count++;
-		op_async->pending = false;
-		gb_loopback_async_operation_put(op_async);
-		gb_loopback_calculate_stats(gb, true);
-	}
 	mutex_unlock(&gb->mutex);
 
-	dev_dbg(&gb->connection->bundle->dev, "timeout operation %d\n",
+	dev_dbg(&gb->connection->bundle->dev, "complete operation %d\n",
 		operation->id);
 
-	gb_operation_cancel(operation, -ETIMEDOUT);
-	gb_loopback_async_operation_put(op_async);
-}
-
-static void gb_loopback_async_operation_timeout(unsigned long data)
-{
-	struct gb_loopback_async_operation *op_async;
-	u16 id = data;
+	/* Wake up waiters */
+	atomic_dec(&op_async->gb->outstanding_operations);
+	wake_up(&gb->wq_completion);
 
-	op_async = gb_loopback_operation_find(id);
-	if (!op_async) {
-		pr_err("operation %d not found - time out ?\n", id);
-		return;
-	}
-	schedule_work(&op_async->work);
+	/* Release resources */
+	gb_operation_put(operation);
+	kfree(op_async);
 }
 
 static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
@@ -596,15 +498,11 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
 	struct gb_loopback_async_operation *op_async;
 	struct gb_operation *operation;
 	int ret;
-	unsigned long flags;
 
 	op_async = kzalloc(sizeof(*op_async), GFP_KERNEL);
 	if (!op_async)
 		return -ENOMEM;
 
-	INIT_WORK(&op_async->work, gb_loopback_async_operation_work);
-	kref_init(&op_async->kref);
-
 	operation = gb_operation_create(gb->connection, type, request_size,
 					response_size, GFP_KERNEL);
 	if (!operation) {
@@ -615,32 +513,29 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
 	if (request_size)
 		memcpy(operation->request->payload, request, request_size);
 
+	gb_operation_set_data(operation, op_async);
+
 	op_async->gb = gb;
 	op_async->operation = operation;
 	op_async->completion = completion;
 
-	spin_lock_irqsave(&gb_dev.lock, flags);
-	list_add_tail(&op_async->entry, &gb_dev.list_op_async);
-	spin_unlock_irqrestore(&gb_dev.lock, flags);
-
 	do_gettimeofday(&op_async->ts);
-	op_async->pending = true;
+
 	atomic_inc(&gb->outstanding_operations);
+
 	mutex_lock(&gb->mutex);
-	ret = gb_operation_request_send(operation,
+	ret = gb_operation_request_send_timeout(operation,
+					jiffies_to_msecs(gb->jiffy_timeout),
 					gb_loopback_async_operation_callback,
 					GFP_KERNEL);
 	if (ret)
 		goto error;
 
-	setup_timer(&op_async->timer, gb_loopback_async_operation_timeout,
-			(unsigned long)operation->id);
-	op_async->timer.expires = jiffies + gb->jiffy_timeout;
-	add_timer(&op_async->timer);
-
 	goto done;
 error:
-	gb_loopback_async_operation_put(op_async);
+	atomic_dec(&gb->outstanding_operations);
+	gb_operation_put(operation);
+	kfree(op_async);
 done:
 	mutex_unlock(&gb->mutex);
 	return ret;
@@ -1045,8 +940,10 @@ static int gb_loopback_fn(void *data)
 				error = gb_loopback_async_sink(gb, size);
 			}
 
-			if (error)
+			if (error) {
 				gb->error++;
+				gb->iteration_count++;
+			}
 		} else {
 			/* We are effectively single threaded here */
 			if (type == GB_LOOPBACK_TYPE_PING)
-- 
2.7.4



More information about the devel mailing list