[openib-general] Re: Re: SDP_CONN_LOCK

Michael S. Tsirkin mst at mellanox.co.il
Sat Feb 19 16:11:37 PST 2005


Quoting r. Libor Michalek <libor at topspin.com>:
> Subject: Re: Re: SDP_CONN_LOCK
> 
> On Thu, Feb 17, 2005 at 03:49:31PM -0800, Sean Hefty wrote:
> > Michael S. Tsirkin wrote:
> > > Quoting r. Roland Dreier <roland at topspin.com>:
> > >>
> > >>BTW, since mthca currently calls completion handlers directly from
> > >>interrupt context (rather than BH/tasklet context), it might be worth
> > >>renaming all the SDP locking macros so they're not confusingly named
> > >>with _BH suffixes.
> > > 
> > > I think it would be much nicer to reduce the number of macros used.
> > 
> > I'd have to agree with this.  The SDP locking macros are fairly complex 
> > and hide a lot of functionality.  E.g. SDP_CONN_RELOCK results in 
> > polling/rearming the CQ, same with SDP_CONN_UNLOCK.  Maybe that's just 
> > a naming issue though.
> > 
> > I think these would probably be better off as just function calls, 
> > rather than macros.  SDP_CONN_LOCK calls sdp_conn_internal_lock(), and 
> > that appears to be the only place that the function is called. 
> > Similarly, SDP_CONN_UNLOCK calls sdp_conn_internal_unlock().  It seems 
> > that you could just merge the macros into the function calls.
> 
>   OK. I had used macros since in the fast path the functions wouldn't
> get called, but since it sounds like it makes things less readable I
> should be able to quickly try out replacing the macros with functions,
> and see what if any effect it has on performance.
> 
>   And here I would have thought SDP_CONN_HOLD as a macro would have
> bugged people more. :)
> 
> -Libor
> 
Didn't get that far yet :)

I also wander whether we could get rid of most called to 
_CHECK macros, consequently turn lots of functions
which cant otherwise fail into void?
Given that these checks are disabled upon a compile switch, one wanders
how big a debugging aid they are.

Consider sdp_wall_recv_accept calling sdp_wall_failedm both checking
their parameter for NULL - why does it make sence?


-- 
MST - Michael S. Tsirkin



More information about the general mailing list