[openib-general] [PATCH] for review to timeout send MADs

[Roland Dreier]

    Sean> A workqueue is used to schedule delayed processing of MAD
    Sean> timeouts.  The scheduling delay of the timeout thread is
    Sean> adjusted when a send completes or is canceled.  If anyone
    Sean> sees an issue with my usage of the workqueue, just let me
    Sean> know.

It seems you are using the system-wide keventd queue.  This isn't
necessarily a problem per se, but it would probably be better to use a
MAD-layer private workqueue (I suggested a single-threaded workqueue
per MAD "port" earlier).  This avoids two problems.  First, keventd is
subject to arbitrary delays because it is used as a dumping ground for
any code that needs to sleep.  Second, if the MAD layer has its own
workqueue, then it can be used for completion processing as well; as
it stands it seems sort of funny to create a kthread to do some work
and run other work from a workqueue.

A few low-level comments too:

 		rbuf = list_entry(&port_priv->recv_posted_mad_list[qpn],
-				 struct ib_mad_recv_buf,
-				 list);
-		rbuf = (struct ib_mad_recv_buf *)rbuf->list.next;
+				  struct ib_mad_recv_buf,
+				  list);
+		rbuf = (struct ib_mad_recv_buf*)rbuf->list.next;

I don't understand what's going on here; can this not be written as:

 		rbuf = list_entry(port_priv->recv_posted_mad_list[qpn].next,
				  struct ib_mad_recv_buf, list);

(By the way the cast should be written with spaces as:
	(struct ib_mad_recv_buf *) rbuf->list.next)

This patch seems whitespace-challenged in other places too:

-		recv = container_of(mad_priv_hdr, struct ib_mad_private, header);
+		recv = container_of(mad_priv_hdr,struct ib_mad_private,header);

and has extra empty lines places like here:

+		if (time_after(mad_agent_priv->timeout, mad_send_wr->timeout)) {
+
+			mad_agent_priv->timeout = mad_send_wr->timeout;

and here:

+	if (list_empty(&mad_agent_priv->wait_list)) {
+		cancel_delayed_work(&mad_agent_priv->work);
+	} else {
+
+		mad_send_wr = list_entry(mad_agent_priv->wait_list.next,