[ofw] RE: IBBUS - keep last HCA reference until IBAL is shutdown.
Smith, Stan
stan.smith at intel.com
Tue May 19 09:22:55 PDT 2009
Hefty, Sean wrote:
>> - if ( ic > 0 && p_ext->hca_ifc_taken ) {
>> + if ( ic > 1 && p_ext->hca_ifc_taken ) {
>
> I realize this is old code, but... can someone explain the logic for
> this (before or after the change)? I would expect to see logic more
> like:
>
> some_count--;
> if (some_count == 0)
> InterfaceDereference();
>
> The && hca_ifc_taken check is somewhat confusing, but only releasing
> the interface reference if ic is positive (or greater than 1 with the
> change) "feels" like the wrong approach.
>
> We don't release the interface if ic = 0, or does ic = 0 imply
> something about value for hca_ifc_taken? If so, can we eliminate
> hca_ifc_taken? If ic = 4, we release the reference immediately, then
> decrement ic to 3. So whatever bumped ic to 1 or 2 or 3 in the first
> place no longer has access to the HCA interface?
>
> (No, I didn't spend any time trying to figure this code out.
> However, IMO, reference counting should be as clear as possible wrt
> what's happening.)
>
> - Sean
The problem with diffs is there is too little context for understanding unless one is familiar with the code. In order to fully understand what's going on, reading the code would be advised.
'hca_ifc_taken' TRUE indicates a reference was taken on the HCA.
'ic' is function local shorthand for the current number of Bus Filter Instances (BFI) (aka enabled HCAs).
The rational behind the code is during HCA disable, do not release the last HCA reference until after IBAL has shutdown.
'ic' has not dropped to zero as the current BFI has not been freed; hence '>1' TRUE == not the last HCA being disabled.
Make sense?
Stan.
More information about the ofw
mailing list