Quoting Sean Hefty <[EMAIL PROTECTED]>: > The state of the cm_id is controlled by the CM and can change at any time > as a result of processing a received MAD.
I see. Lets hide this field then. At least, this warrants a comment in the header file. > It's only exposed for debug purposes. I'd say you cant usefully debug with something that changes at any time, anyway. Let's just have a compile time flag for dumping cm traffic to syslog. Makes sense? [...] > >I'd prefer separate labels for two errors, instead of testing IS_ERR twice. > > So do I, but I was trying to match the coding style used throughout the SDP > code. Fixing the error handling seems like another set of changes to me. Lets do it correctly in this function, no need to add more cleanup work. > >Regarding the question: the first thing we do in sdp_conn_put > >is sdp_conn_table_remove, so I think this lookup can fail. > > I should have removed that comment. I'm not overly familiar with the SDP > code, but it seems wrong to need to verify that a requested callback can be > executed. By returning -EINVAL above, the cm_id associated with the > callback will be destroyed. This indicates to me that either that cm_id > will be destroyed twice by SDP (once when the SDP conn object is destroyed > and again here), or that SDP does not usually destroy cm_id's as part of > its normal cleanup. Hmm. Got to think about it. > I'm also wondering about possible race conditions that > could occur between calling table_lookup and conn_lock, but I don't know > the code well enough to say if one exists. No, conn_lock does not handle connection reference counting, it only protects against two threads running on a connection. -- MST _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
