[openib-general] [PATCH] for review to timeout send MADs
Sean Hefty
sean.hefty at intel.com
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.
More information about the general
mailing list