| Submitter | roel kluin |
|---|---|
| Date | Feb. 18, 2009, 12:38 p.m. |
| Message ID | <499C0143.1080703@gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/23341/ |
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Comments
Reviewed-by: Steve Wise <swise@opengridcomputing.com> Roel Kluin wrote: > Dropped general@lists.openfabrics.org since it bounces, and I missed one > in my previously sent patch > --------------------------->8-------------8<------------------------------ > Logical-/bit-or typo > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com> > --- > diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c > index 44e936e..21c6145 100644 > --- a/drivers/infiniband/hw/cxgb3/iwch_cm.c > +++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c > @@ -890,7 +890,7 @@ static void process_mpa_reply(struct iwch_ep *ep, struct sk_buff *skb) > */ > state_set(&ep->com, FPDU_MODE); > ep->mpa_attr.initiator = 1; > - ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) | crc_enabled ? 1 : 0; > + ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) || crc_enabled ? 1 : 0; > ep->mpa_attr.recv_marker_enabled = markers_enabled; > ep->mpa_attr.xmit_marker_enabled = mpa->flags & MPA_MARKERS ? 1 : 0; > ep->mpa_attr.version = mpa_rev; > @@ -1013,7 +1013,7 @@ static void process_mpa_request(struct iwch_ep *ep, struct sk_buff *skb) > * start reply message including private data. > */ > ep->mpa_attr.initiator = 0; > - ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) | crc_enabled ? 1 : 0; > + ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) || crc_enabled ? 1 : 0; > ep->mpa_attr.recv_marker_enabled = markers_enabled; > ep->mpa_attr.xmit_marker_enabled = mpa->flags & MPA_MARKERS ? 1 : 0; > ep->mpa_attr.version = mpa_rev; > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> - ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) | crc_enabled ? 1 : 0; > + ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) || crc_enabled ? 1 : 0; > - ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) | crc_enabled ? 1 : 0; > + ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) || crc_enabled ? 1 : 0; So as I said before, I guess this change is fine, except: add/remove: 0/0 grow/shrink: 1/0 up/down: 18/0 (18) function old new delta rx_data 1237 1255 +18 ie it makes the object code 18 bytes bigger on x86-64 (gcc 4.3.2). Given that the code works the same either way, is this change a net win? - R. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Roland Dreier wrote: > > - ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) | crc_enabled ? 1 : 0; > > + ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) || crc_enabled ? 1 : 0; > > > - ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) | crc_enabled ? 1 : 0; > > + ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) || crc_enabled ? 1 : 0; > > So as I said before, I guess this change is fine, except: > > add/remove: 0/0 grow/shrink: 1/0 up/down: 18/0 (18) > function old new delta > rx_data 1237 1255 +18 > > ie it makes the object code 18 bytes bigger on x86-64 (gcc 4.3.2). > Given that the code works the same either way, is this change a net win? > > - R. > It does not bother me to leave the code as-is. Roel, Was this found by code inspection or some tool that is run on the code? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c index 44e936e..21c6145 100644 --- a/drivers/infiniband/hw/cxgb3/iwch_cm.c +++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c @@ -890,7 +890,7 @@ static void process_mpa_reply(struct iwch_ep *ep, struct sk_buff *skb) */ state_set(&ep->com, FPDU_MODE); ep->mpa_attr.initiator = 1; - ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) | crc_enabled ? 1 : 0; + ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) || crc_enabled ? 1 : 0; ep->mpa_attr.recv_marker_enabled = markers_enabled; ep->mpa_attr.xmit_marker_enabled = mpa->flags & MPA_MARKERS ? 1 : 0; ep->mpa_attr.version = mpa_rev; @@ -1013,7 +1013,7 @@ static void process_mpa_request(struct iwch_ep *ep, struct sk_buff *skb) * start reply message including private data. */ ep->mpa_attr.initiator = 0; - ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) | crc_enabled ? 1 : 0; + ep->mpa_attr.crc_enabled = (mpa->flags & MPA_CRC) || crc_enabled ? 1 : 0; ep->mpa_attr.recv_marker_enabled = markers_enabled; ep->mpa_attr.xmit_marker_enabled = mpa->flags & MPA_MARKERS ? 1 : 0; ep->mpa_attr.version = mpa_rev;
Dropped general@lists.openfabrics.org since it bounces, and I missed one in my previously sent patch --------------------------->8-------------8<------------------------------ Logical-/bit-or typo Signed-off-by: Roel Kluin <roel.kluin@gmail.com> --- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html