netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] RDMA/cxgb3: logical-/bit-or confusion?
       [not found] <499BD470.4080705@gmail.com>
@ 2009-02-18 12:38 ` Roel Kluin
  2009-02-18 15:13   ` Steve Wise
  2009-02-19  0:37   ` Roland Dreier
  0 siblings, 2 replies; 4+ messages in thread
From: Roel Kluin @ 2009-02-18 12:38 UTC (permalink / raw)
  To: swise; +Cc: Andrew Morton, David S. Miller, netdev

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;

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] RDMA/cxgb3: logical-/bit-or confusion?
  2009-02-18 12:38 ` [PATCH] RDMA/cxgb3: logical-/bit-or confusion? Roel Kluin
@ 2009-02-18 15:13   ` Steve Wise
  2009-02-19  0:37   ` Roland Dreier
  1 sibling, 0 replies; 4+ messages in thread
From: Steve Wise @ 2009-02-18 15:13 UTC (permalink / raw)
  To: Roel Kluin; +Cc: Andrew Morton, David S. Miller, netdev


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
>   


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] RDMA/cxgb3: logical-/bit-or confusion?
  2009-02-18 12:38 ` [PATCH] RDMA/cxgb3: logical-/bit-or confusion? Roel Kluin
  2009-02-18 15:13   ` Steve Wise
@ 2009-02-19  0:37   ` Roland Dreier
  2009-02-19  0:53     ` Steve Wise
  1 sibling, 1 reply; 4+ messages in thread
From: Roland Dreier @ 2009-02-19  0:37 UTC (permalink / raw)
  To: Roel Kluin; +Cc: swise, Andrew Morton, David S. Miller, netdev

 > -	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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] RDMA/cxgb3: logical-/bit-or confusion?
  2009-02-19  0:37   ` Roland Dreier
@ 2009-02-19  0:53     ` Steve Wise
  0 siblings, 0 replies; 4+ messages in thread
From: Steve Wise @ 2009-02-19  0:53 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Roel Kluin, Andrew Morton, David S. Miller, netdev

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?


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-02-19  0:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <499BD470.4080705@gmail.com>
2009-02-18 12:38 ` [PATCH] RDMA/cxgb3: logical-/bit-or confusion? Roel Kluin
2009-02-18 15:13   ` Steve Wise
2009-02-19  0:37   ` Roland Dreier
2009-02-19  0:53     ` Steve Wise

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).