| Message ID | 1361437377.1341.33.camel@x61.thuisdomein (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Thursday 21 February 2013 11:02, Paul Bolle wrote: > diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c > index 19e0637..512fde3 100644 > --- a/drivers/infiniband/hw/mlx4/qp.c > +++ b/drivers/infiniband/hw/mlx4/qp.c > @@ -1778,8 +1778,8 @@ static int build_mlx_header(struct mlx4_ib_sqp *sqp, struct ib_send_wr *wr, > } > > vlan = rdma_get_vlan_id(&sgid); > - is_vlan = vlan < 0x1000; > } Nice try! However, this approach does add the line below to processing for an IB port (ETH/RoCE port stays same, more or less). Processing time is therefore increased (at least on the IB side) relative to just living with the warning. Roland? > + is_vlan = vlan < 0x1000; <=== Code line added to IB-side processing. > ib_ud_header_init(send_size, !is_eth, is_eth, is_vlan, is_grh, 0, &sqp->ud_header); > > if (!is_eth) { -Jack -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 24, 2013 at 4:34 AM, Jack Morgenstein <jackm@dev.mellanox.co.il> wrote: > However, this approach does add the line below to processing for an IB port (ETH/RoCE port stays same, more or less). > Processing time is therefore increased (at least on the IB side) relative to just living with the warning. > > Roland? I'm finally noticing that this is in the build_mlx_header() function, which is pretty much a slow path. Certainly another compare isn't going to change performance given all the other stuff we do there. Let me look at the patches that have gone by and see what the cleanest way to handle this is. - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 25, 2013 at 8:54 AM, Roland Dreier <roland@kernel.org> wrote: > I'm finally noticing that this is in the build_mlx_header() function, > which is pretty much a slow path. Certainly another compare isn't > going to change performance given all the other stuff we do there. > > Let me look at the patches that have gone by and see what the cleanest > way to handle this is. OK, after playing around a bit, I see that just initializing vlan doesn't really change the generated code (my gcc at least was already if effect setting vlan in the generated assembly code), so I'll just merge that. - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 25 February 2013 19:23, Roland Dreier wrote: > On Mon, Feb 25, 2013 at 8:54 AM, Roland Dreier <roland@kernel.org> wrote: > > I'm finally noticing that this is in the build_mlx_header() function, > > which is pretty much a slow path. Certainly another compare isn't > > going to change performance given all the other stuff we do there. > > > > Let me look at the patches that have gone by and see what the cleanest > > way to handle this is. > > OK, after playing around a bit, I see that just initializing vlan > doesn't really change the generated code (my gcc at least was already > if effect setting vlan in the generated assembly code), so I'll just > merge that. > > - R. Thanks! -Jack -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c index 19e0637..512fde3 100644 --- a/drivers/infiniband/hw/mlx4/qp.c +++ b/drivers/infiniband/hw/mlx4/qp.c @@ -1747,9 +1747,9 @@ static int build_mlx_header(struct mlx4_ib_sqp *sqp, struct ib_send_wr *wr, int spc; int i; int is_eth; - int is_vlan = 0; + int is_vlan; int is_grh; - u16 vlan; + u16 vlan = 0xffff; /* invalid vlan id */ int err = 0; send_size = 0; @@ -1778,8 +1778,8 @@ static int build_mlx_header(struct mlx4_ib_sqp *sqp, struct ib_send_wr *wr, } vlan = rdma_get_vlan_id(&sgid); - is_vlan = vlan < 0x1000; } + is_vlan = vlan < 0x1000; ib_ud_header_init(send_size, !is_eth, is_eth, is_vlan, is_grh, 0, &sqp->ud_header); if (!is_eth) {
Building qp.o (part of the "Mellanox ConnectX HCA support" driver) triggers this GCC warning: drivers/infiniband/hw/mlx4/qp.c: In function ‘mlx4_ib_post_send’: drivers/infiniband/hw/mlx4/qp.c:1862:30: warning: ‘vlan’ may be used uninitialized in this function [-Wmaybe-uninitialized] drivers/infiniband/hw/mlx4/qp.c:1752:6: note: ‘vlan’ was declared here Looking at the code it is clear 'vlan' is only set and used if 'is_eth' is non-zero. But by, basically, initializing 'vlan' to 0xffff, instead of the equivalent of initializing 'is_vlan' to zero, we can make this warning go away. Signed-off-by: Paul Bolle <pebolle@tiscali.nl> --- 0) Jack wanted to use uninitialized_var() after I posted the first version of this trivial patch. I suggested to see what happened with Ingo's idea to remove uninitialized_var() entirely. Nothing seems to have happened, so I decided to try a different approach. 1) Still compile tested only, but now against v3.8. drivers/infiniband/hw/mlx4/qp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)