Hal Rosenstock wrote:
> Hi Yosef,
>
> On Mon, 2007-05-07 at 10:18, Yosef Etigin wrote:
>
>>Michael S. Tsirkin wrote:
>>
>>>>@@ -1865,6 +1863,15 @@ static void ib_mad_recv_done_handler(str
>>>> recv->header.recv_wc.recv_buf.mad = &recv->mad.mad;
>>>> recv->header.recv_wc.recv_buf.grh = &recv->grh;
>>>>
>>>>+ /* update our lmc cache with port info smps */
>>>>+ if ((recv->mad.mad.mad_hdr.mgmt_class == IB_MGMT_CLASS_SUBN_LID_ROUTED
>>>>||
>>>>+ recv->mad.mad.mad_hdr.mgmt_class ==
>>>>IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE)
>>>>+ && (recv->mad.mad.mad_hdr.attr_id == IB_SMP_ATTR_PORT_INFO)
>>>>+ && (recv->mad.mad.mad_hdr.method == IB_MGMT_METHOD_SET))
>>>>+ {
>>>>+ atomic_set(&port_priv->port_lmc, recv->mad.smp.data[34] & 0x7);
>>>>+ }
>>>>+
>>>> if (atomic_read(&qp_info->snoop_count))
>>>> snoop_recv(qp_info, &recv->header.recv_wc, IB_MAD_SNOOP_RECVS);
>>>>
>>>
>>>
>>>Why is this an atomic?
>>
>>I thought there might be a race between this and where we read the lmc
>>(rcv_has_same_gid)
>>
>>
>>>The comment does not seem to tell us anything useful. Remove it?
>>>These 8 lines seem to violate coding style rules in at least 3 different
>>>ways::)
>>>
>>
>> if ((recv->mad.mad.mad_hdr.mgmt_class == IB_MGMT_CLASS_SUBN_LID_ROUTED
>> ||
>> recv->mad.mad.mad_hdr.mgmt_class ==
>> IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE)
>> && (recv->mad.mad.mad_hdr.attr_id == IB_SMP_ATTR_PORT_INFO)
>> && (recv->mad.mad.mad_hdr.method == IB_MGMT_METHOD_SET))
>> atomic_set(&port_priv->port_lmc, recv->mad.smp.data[34] & 0x7);
>
>
> Should at least a #define be used for smp.data[34} if not a struct so it
> is clearer what is going on here ?
>
you mean something like:
#define LMC_FROM_PORT_INFO(data) ( ( (char*)(data) )[34] & 0x07 ) ?
> I haven't yet had a chance to look at the rest of the patch.
>
> -- Hal
>
>
>>is that better?
>>_______________________________________________
>>general mailing list
>>general@lists.openfabrics.org
>>http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>>
>>To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>
>
_______________________________________________
general mailing list
general@lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
