Re: [PATCH V2 5/9] IB/mlx5: Mellanox Connect-IB, IB driver part 1/5
On Thursday 04 July 2013 16:15, Jack Morgenstein wrote: > > > + *inlen = sizeof **cqb + sizeof *(*cqb)->pas * ncont; > > > > sizeof always uses parentheses > I'll fix this, too. -Jack -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 5/9] IB/mlx5: Mellanox Connect-IB, IB driver part 1/5
On Wednesday 03 July 2013 23:59, Joe Perches wrote:
> On Wed, 2013-07-03 at 20:13 +0300, Or Gerlitz wrote:
> > From: Eli Cohen
> > diff --git a/drivers/infiniband/hw/mlx5/ah.c
> > b/drivers/infiniband/hw/mlx5/ah.c
> []
> > +struct ib_ah *create_ib_ah(struct ib_ah_attr *ah_attr,
> > + struct mlx5_ib_ah *ah)
> > +{
> > + u32 sgi;
>
> sgi is used once here and looks more confusing than helpful
>
Will fix in V3
>
> []
>
> > +static void *get_sw_cqe(struct mlx5_ib_cq *cq, int n)
> > +{
> > + void *cqe = get_cqe(cq, n & cq->ibcq.cqe);
> > + struct mlx5_cqe64 *cqe64;
> > +
> > + cqe64 = (cq->mcq.cqe_sz == 64) ? cqe : cqe + 64;
> > + return ((cqe64->op_own & MLX5_CQE_OWNER_MASK) ^
> > + !!(n & (cq->ibcq.cqe + 1))) ? NULL : cqe;
>
> I think "foo ^ !!bar" is excessively tricky.
>
The mlx4 driver already uses "!!foo ^ !!bar" in several places in the kernel
(and this is old code).
I assume that your problem with the above code is that it uses "foo" and not
"!!foo".
Please note, though, that this code is data-path code -- and in this specific
case, foo = !!foo
(since MLX5_CQE_OWNER_MASK = 1). We decided, therefore, in this specific case,
not to add the unnecessary
"!!", even though at first glance it may look tricky -- performance here IMHO
is more important.
> > +static enum ib_wc_opcode get_umr_comp(struct mlx5_ib_wq *wq, int idx)
> > +{
>
> > + pr_warn("unkonwn completion status\n");
>
> unknown tyop
Will fix in V3
> []
>
> > +static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
> > + struct ib_ucontext *context, struct mlx5_ib_cq *cq,
> > + int entries, struct mlx5_create_cq_mbox_in **cqb,
> > + int *cqe_size, int *index, int *inlen)
> []
> > + *inlen = sizeof **cqb + sizeof *(*cqb)->pas * ncont;
>
> sizeof always uses parentheses
>
> > + *cqb = vzalloc(*inlen);
>
> Perhaps you may be using vzalloc too often.
>
> Maybe you should have a helper allocating either
> from kmalloc or vmalloc as necessary based on size.
We will look into this.
Thanks for reviewing!
-Jack
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 5/9] IB/mlx5: Mellanox Connect-IB, IB driver part 1/5
On Wed, 2013-07-03 at 20:13 +0300, Or Gerlitz wrote:
> From: Eli Cohen
more trivia:
> diff --git a/drivers/infiniband/hw/mlx5/ah.c b/drivers/infiniband/hw/mlx5/ah.c
[]
> +struct ib_ah *create_ib_ah(struct ib_ah_attr *ah_attr,
> +struct mlx5_ib_ah *ah)
> +{
> + u32 sgi;
sgi is used once here and looks more confusing than helpful
> +
> + if (ah_attr->ah_flags & IB_AH_GRH) {
> + sgi = ah_attr->grh.sgid_index << 20;
> +
> + memcpy(ah->av.rgid, &ah_attr->grh.dgid, 16);
> + ah->av.grh_gid_fl = cpu_to_be32(ah_attr->grh.flow_label |
> + (1 << 30) | sgi);
> + ah->av.hop_limit = ah_attr->grh.hop_limit;
> + ah->av.tclass = ah_attr->grh.traffic_class;
> + }
> +
> + ah->av.rlid = cpu_to_be16(ah_attr->dlid);
> + ah->av.fl_mlid = ah_attr->src_path_bits & 0x7f;
> + ah->av.stat_rate_sl = (ah_attr->static_rate << 4) | (ah_attr->sl & 0xf);
> +
> + return &ah->ibah;
> +}
[]
> +static void *get_sw_cqe(struct mlx5_ib_cq *cq, int n)
> +{
> + void *cqe = get_cqe(cq, n & cq->ibcq.cqe);
> + struct mlx5_cqe64 *cqe64;
> +
> + cqe64 = (cq->mcq.cqe_sz == 64) ? cqe : cqe + 64;
> + return ((cqe64->op_own & MLX5_CQE_OWNER_MASK) ^
> + !!(n & (cq->ibcq.cqe + 1))) ? NULL : cqe;
I think "foo ^ !!bar" is excessively tricky.
> +static enum ib_wc_opcode get_umr_comp(struct mlx5_ib_wq *wq, int idx)
> +{
> + pr_warn("unkonwn completion status\n");
unknown tyop
[]
> +static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
> + struct ib_ucontext *context, struct mlx5_ib_cq *cq,
> + int entries, struct mlx5_create_cq_mbox_in **cqb,
> + int *cqe_size, int *index, int *inlen)
[]
> + *inlen = sizeof **cqb + sizeof *(*cqb)->pas * ncont;
sizeof always uses parentheses
> + *cqb = vzalloc(*inlen);
Perhaps you may be using vzalloc too often.
Maybe you should have a helper allocating either
from kmalloc or vmalloc as necessary based on size.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 5/9] IB/mlx5: Mellanox Connect-IB, IB driver part 1/5
From: Eli Cohen
Signed-off-by: Eli Cohen
---
drivers/infiniband/hw/mlx5/ah.c | 95
drivers/infiniband/hw/mlx5/cq.c | 844 +
drivers/infiniband/hw/mlx5/doorbell.c | 100
drivers/infiniband/hw/mlx5/mad.c | 139 ++
4 files changed, 1178 insertions(+), 0 deletions(-)
create mode 100644 drivers/infiniband/hw/mlx5/ah.c
create mode 100644 drivers/infiniband/hw/mlx5/cq.c
create mode 100644 drivers/infiniband/hw/mlx5/doorbell.c
create mode 100644 drivers/infiniband/hw/mlx5/mad.c
diff --git a/drivers/infiniband/hw/mlx5/ah.c b/drivers/infiniband/hw/mlx5/ah.c
new file mode 100644
index 000..ff8f1cb
--- /dev/null
+++ b/drivers/infiniband/hw/mlx5/ah.c
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2013, Mellanox Technologies inc. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer in the documentation and/or other materials
+ *provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "mlx5_ib.h"
+
+struct ib_ah *create_ib_ah(struct ib_ah_attr *ah_attr,
+ struct mlx5_ib_ah *ah)
+{
+ u32 sgi;
+
+ if (ah_attr->ah_flags & IB_AH_GRH) {
+ sgi = ah_attr->grh.sgid_index << 20;
+
+ memcpy(ah->av.rgid, &ah_attr->grh.dgid, 16);
+ ah->av.grh_gid_fl = cpu_to_be32(ah_attr->grh.flow_label |
+ (1 << 30) | sgi);
+ ah->av.hop_limit = ah_attr->grh.hop_limit;
+ ah->av.tclass = ah_attr->grh.traffic_class;
+ }
+
+ ah->av.rlid = cpu_to_be16(ah_attr->dlid);
+ ah->av.fl_mlid = ah_attr->src_path_bits & 0x7f;
+ ah->av.stat_rate_sl = (ah_attr->static_rate << 4) | (ah_attr->sl & 0xf);
+
+ return &ah->ibah;
+}
+
+struct ib_ah *mlx5_ib_create_ah(struct ib_pd *pd, struct ib_ah_attr *ah_attr)
+{
+ struct mlx5_ib_ah *ah;
+
+ ah = kzalloc(sizeof(*ah), GFP_ATOMIC);
+ if (!ah)
+ return ERR_PTR(-ENOMEM);
+
+ return create_ib_ah(ah_attr, ah); /* never fails */
+}
+
+int mlx5_ib_query_ah(struct ib_ah *ibah, struct ib_ah_attr *ah_attr)
+{
+ struct mlx5_ib_ah *ah = to_mah(ibah);
+ u32 tmp;
+
+ memset(ah_attr, 0, sizeof(*ah_attr));
+
+ tmp = be32_to_cpu(ah->av.grh_gid_fl);
+ if (tmp & (1 << 30)) {
+ ah_attr->ah_flags = IB_AH_GRH;
+ ah_attr->grh.sgid_index = (tmp >> 20) & 0xff;
+ ah_attr->grh.flow_label = tmp & 0xf;
+ memcpy(&ah_attr->grh.dgid, ah->av.rgid, 16);
+ ah_attr->grh.hop_limit = ah->av.hop_limit;
+ ah_attr->grh.traffic_class = ah->av.tclass;
+ }
+ ah_attr->dlid = be16_to_cpu(ah->av.rlid);
+ ah_attr->static_rate = ah->av.stat_rate_sl >> 4;
+ ah_attr->sl = ah->av.stat_rate_sl & 0xf;
+
+ return 0;
+}
+
+int mlx5_ib_destroy_ah(struct ib_ah *ah)
+{
+ kfree(to_mah(ah));
+ return 0;
+}
diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
new file mode 100644
index 000..c05868e
--- /dev/null
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -0,0 +1,844 @@
+/*
+ * Copyright (c) 2013, Mellanox Technologies inc. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must
