greybus: simplify pending operations tracking

Greg raised the alarm when I first put in the red-black tree for
tracking pending operations.  The reality as that we're not likely
to have that many operations in flight at any one time, so the
complexity of the red-black tree is most likely unwarranted.  I
already

This pulls out the red-black tree and uses a simple list instead.  A
connection maintains two lists of operations.  An operation starts
on its connection's operations list.  It is moved to the pending
list when its request message is sent.  And it is moved back to
the operations list when the response message arrives.  It is
removed from whatever list it's in when the operation is destroyed.
We reuse the single operation->links field for both lists.

Only outgoing requests are ever "pending."  Incoming requests are
transient--we receive them, process them, send the response, and
then we're done.

Change a few function names so it's clear we're working with the
pending list.

Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c
index 586457f027de5dfc0d4de9310896cde760354178..377ac7d3afea5a59a670c0d8643030d863e71270 100644 (file)
--- a/drivers/staging/greybus/connection.c
+++ b/drivers/staging/greybus/connection.c
@@ -209,7 +209,7 @@ struct gb_connection *gb_connection_create(struct gb_interface *interface,
        spin_unlock_irq(&gb_connections_lock);
 
        INIT_LIST_HEAD(&connection->operations);
-       connection->pending = RB_ROOT;
+       INIT_LIST_HEAD(&connection->pending);
        atomic_set(&connection->op_cycle, 0);
 
        return connection;

diff --git a/drivers/staging/greybus/connection.h b/drivers/staging/greybus/connection.h
index 893c02af371053eadc5a725fa91a738ee1560919..861e0661157cba514914027759bb75940109c298 100644 (file)
--- a/drivers/staging/greybus/connection.h
+++ b/drivers/staging/greybus/connection.h
@@ -36,7 +36,7 @@ struct gb_connection {
        enum gb_connection_state        state;
 
        struct list_head                operations;
-       struct rb_root                  pending;        /* awaiting reponse */
+       struct list_head                pending;        /* awaiting reponse */
        atomic_t                        op_cycle;
 
        void                            *private;

diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
index 5f3e52d8d485a3ef8109bc19956bf7d33cb4ef4e..cc492108134511077a4aeabc33f24a6895d3543e 100644 (file)
--- a/drivers/staging/greybus/operation.c
+++ b/drivers/staging/greybus/operation.c
@@ -56,13 +56,9 @@ struct gb_operation_msg_hdr {
 /* XXX Could be per-host device, per-module, or even per-connection */
 static DEFINE_SPINLOCK(gb_operations_lock);
 
-static void gb_operation_insert(struct gb_operation *operation)
+static void gb_pending_operation_insert(struct gb_operation *operation)
 {
        struct gb_connection *connection = operation->connection;
-       struct rb_root *root = &connection->pending;
-       struct rb_node *node = &operation->node;
-       struct rb_node **link = &root->rb_node;
-       struct rb_node *above = NULL;
        struct gb_operation_msg_hdr *header;
 
        /* Assign the operation's id, and store it in the header of
@@ -72,25 +68,13 @@ static void gb_operation_insert(struct gb_operation *operation)
        header = operation->request->transfer_buffer;
        header->id = cpu_to_le16(operation->id);
 
-       /* OK, insert the operation into its connection's pending tree */
+       /* Insert the operation into its connection's pending list */
        spin_lock_irq(&gb_operations_lock);
-       while (*link) {
-               struct gb_operation *other;
-
-               above = *link;
-               other = rb_entry(above, struct gb_operation, node);
-               header = other->request->transfer_buffer;
-               if (other->id > operation->id)
-                       link = &above->rb_left;
-               else if (other->id < operation->id)
-                       link = &above->rb_right;
-       }
-       rb_link_node(node, above, link);
-       rb_insert_color(node, root);
+       list_move_tail(&operation->links, &connection->pending);
        spin_unlock_irq(&gb_operations_lock);
 }
 
-static void gb_operation_remove(struct gb_operation *operation)
+static void gb_pending_operation_remove(struct gb_operation *operation)
 {
        struct gb_connection *connection = operation->connection;
 
@@ -99,29 +83,22 @@ static void gb_operation_remove(struct gb_operation *operation)
 
        /* Take us off of the list of pending operations */
        spin_lock_irq(&gb_operations_lock);
-       rb_erase(&operation->node, &connection->pending);
+       list_move_tail(&operation->links, &connection->operations);
        spin_unlock_irq(&gb_operations_lock);
-
 }
 
 static struct gb_operation *
-gb_operation_find(struct gb_connection *connection, u16 id)
+gb_pending_operation_find(struct gb_connection *connection, u16 id)
 {
-       struct gb_operation *operation = NULL;
-       struct rb_node *node;
+       struct gb_operation *operation;
        bool found = false;
 
        spin_lock_irq(&gb_operations_lock);
-       node = connection->pending.rb_node;
-       while (node && !found) {
-               operation = rb_entry(node, struct gb_operation, node);
-               if (operation->id > id)
-                       node = node->rb_left;
-               else if (operation->id < id)
-                       node = node->rb_right;
-               else
+       list_for_each_entry(operation, &connection->pending, links)
+               if (operation->id == id) {
                        found = true;
-       }
+                       break;
+               }
        spin_unlock_irq(&gb_operations_lock);
 
        return found ? operation : NULL;
@@ -405,7 +382,7 @@ int gb_operation_request_send(struct gb_operation *operation,
         * setting the operation id and submitting the gbuf.
         */
        operation->callback = callback;
-       gb_operation_insert(operation);
+       gb_pending_operation_insert(operation);
        ret = greybus_submit_gbuf(operation->request, GFP_KERNEL);
        if (ret)
                return ret;
@@ -424,7 +401,6 @@ int gb_operation_request_send(struct gb_operation *operation,
  */
 int gb_operation_response_send(struct gb_operation *operation)
 {
-       gb_operation_remove(operation);
        gb_operation_destroy(operation);
 
        return 0;
@@ -456,12 +432,12 @@ void gb_connection_operation_recv(struct gb_connection *connection,
        if (header->type & GB_OPERATION_TYPE_RESPONSE) {
                u16 id = le16_to_cpu(header->id);
 
-               operation = gb_operation_find(connection, id);
+               operation = gb_pending_operation_find(connection, id);
                if (!operation) {
                        gb_connection_err(connection, "operation not found");
                        return;
                }
-               gb_operation_remove(operation);
+               gb_pending_operation_remove(operation);
                gbuf = operation->response;
                gbuf->status = GB_OP_SUCCESS;   /* If we got here we're good */
                if (size > gbuf->transfer_buffer_length) {

diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h
index 965ad9c1aa8c23b5c55ba073e16e1366ab2a9856..4913f720a7cd028f5e385b9eae258331554f5462 100644 (file)
--- a/drivers/staging/greybus/operation.h
+++ b/drivers/staging/greybus/operation.h
@@ -64,8 +64,7 @@ struct gb_operation {
        struct completion       completion;     /* Used if no callback */
        struct delayed_work     timeout_work;
 
-       struct list_head        links;          /* connection->operations */
-       struct rb_node          node;           /* connection->pending */
+       struct list_head        links;  /* connection->{operations,pending} */
 
        /* These are what's used by caller */
        void                    *request_payload;