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

Wed Oct 6 21:10:00 PDT 2004

>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.

I am using the system level queue.  If we think that using our own MAD queue
is better, I will do that.  I was thinking more along the lines of a single
workqueue for all MAD services, with one per processor, rather than a
workqueue per port, however. 

I was planning on changing completion processing to using work queues in a
separate patch.

>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);
>

What happened is that I noticed that list_entry was being used like you saw
and went to correct it the way you suggested, but backed out that change
(manually) after I saw the problem in another area to avoid combining
patches.  I missed that I didn't revert to the original code.  I've started
a separate patch to fix-up this issue, and will make sure that this patch
does not modify this area of the code.

>This patch seems whitespace-challenged in other places too:

I'll go back and update this.  Thanks.  If you see other places that I
missed other than what you mentioned, please let me know.