> Quoting Yosef Etigin <[EMAIL PROTECTED]>:
> Subject: Re: [PATCH 5/6 v2] fix pkey change handling and remove the cahce
>
> 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)
Aren't all incoming MADs on a port handled over a single threaded WQ?
And how would atomics help?
> > 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);
>
> is that better?
Move && to the end of each line, and kill the extra () around single
comparisons.
--
MST
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
