[PATCH] ib/mlx4: add IB_CQ_REPORT_MISSED_EVENTS support

[PATCH] ib/mlx4: add IB_CQ_REPORT_MISSED_EVENTS support

[Or Gerlitz]

enhance the cq arming code to support IB_CQ_REPORT_MISSED_EVENTS

Signed-off-by: Or Gerlitz <ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org>

----

I noted that the IB_CQ_REPORT_MISSED_EVENTS flag was added in the same cycle with mlx4
and maybe as of this, mlx4 didn't implement the flag, which is used by IPoIB

The patch is compile tested only, if the patch seems okay, I can conduct further testing.

diff --git a/drivers/infiniband/hw/mlx4/cq.c b/drivers/infiniband/hw/mlx4/cq.c
index 5a219a2..4366811 100644
--- a/drivers/infiniband/hw/mlx4/cq.c
+++ b/drivers/infiniband/hw/mlx4/cq.c
@@ -755,6 +755,13 @@ int mlx4_ib_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
 		    to_mdev(ibcq->device)->uar_map,
 		    MLX4_GET_DOORBELL_LOCK(&to_mdev(ibcq->device)->uar_lock));
 
+	if (flags & IB_CQ_REPORT_MISSED_EVENTS) {
+		struct mlx4_cqe *cqe;
+		cqe = next_cqe_sw(to_mcq(ibcq));
+		if (cqe)
+			return 1;
+	}
+
 	return 0;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib/mlx4: add IB_CQ_REPORT_MISSED_EVENTS support

[Eli Cohen]

I don't think this patch is required for mlx4.

         < 0    means an error occurred while requesting notification
        == 0    means notification was requested successfully, and if
                IB_CQ_REPORT_MISSED_EVENTS was passed in, then no
                events were missed and it is safe to wait for another
                event.
         > 0    is only returned if IB_CQ_REPORT_MISSED_EVENTS was
                passed in.  It means that the consumer must poll the
                CQ again to make sure it is empty to avoid the race
                described above.

returning 1 means that you must poll the CQ to avoid a race condition
which is not true for mlx4. For example if you return always 0 than
you don't violate what the changelog says.

On Tue, Jul 27, 2010 at 11:21 AM, Or Gerlitz <ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> enhance the cq arming code to support IB_CQ_REPORT_MISSED_EVENTS
>
> Signed-off-by: Or Gerlitz <ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org>
>
> ----
>
> I noted that the IB_CQ_REPORT_MISSED_EVENTS flag was added in the same cycle with mlx4
> and maybe as of this, mlx4 didn't implement the flag, which is used by IPoIB
>
> The patch is compile tested only, if the patch seems okay, I can conduct further testing.
>
> diff --git a/drivers/infiniband/hw/mlx4/cq.c b/drivers/infiniband/hw/mlx4/cq.c
> index 5a219a2..4366811 100644
> --- a/drivers/infiniband/hw/mlx4/cq.c
> +++ b/drivers/infiniband/hw/mlx4/cq.c
> @@ -755,6 +755,13 @@ int mlx4_ib_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
>                    to_mdev(ibcq->device)->uar_map,
>                    MLX4_GET_DOORBELL_LOCK(&to_mdev(ibcq->device)->uar_lock));
>
> +       if (flags & IB_CQ_REPORT_MISSED_EVENTS) {
> +               struct mlx4_cqe *cqe;
> +               cqe = next_cqe_sw(to_mcq(ibcq));
> +               if (cqe)
> +                       return 1;
> +       }
> +
>        return 0;
>  }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib/mlx4: add IB_CQ_REPORT_MISSED_EVENTS support

[Bart Van Assche]

Can I conclude from this that the polling loop (2) from
http://www.mail-archive.com/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg04907.html
won't trigger a race on a multiprocessor when using mlx4 hardware ?

Bart.

On Tue, Jul 27, 2010 at 11:14 AM, Eli Cohen <eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>
> I don't think this patch is required for mlx4.
>
>         < 0    means an error occurred while requesting notification
>        == 0    means notification was requested successfully, and if
>                IB_CQ_REPORT_MISSED_EVENTS was passed in, then no
>                events were missed and it is safe to wait for another
>                event.
>         > 0    is only returned if IB_CQ_REPORT_MISSED_EVENTS was
>                passed in.  It means that the consumer must poll the
>                CQ again to make sure it is empty to avoid the race
>                described above.
>
> returning 1 means that you must poll the CQ to avoid a race condition
> which is not true for mlx4. For example if you return always 0 than
> you don't violate what the changelog says.
>
> On Tue, Jul 27, 2010 at 11:21 AM, Or Gerlitz <ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> > enhance the cq arming code to support IB_CQ_REPORT_MISSED_EVENTS
> >
> > Signed-off-by: Or Gerlitz <ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org>
> >
> > ----
> >
> > I noted that the IB_CQ_REPORT_MISSED_EVENTS flag was added in the same cycle with mlx4
> > and maybe as of this, mlx4 didn't implement the flag, which is used by IPoIB
> >
> > The patch is compile tested only, if the patch seems okay, I can conduct further testing.
> >
> > diff --git a/drivers/infiniband/hw/mlx4/cq.c b/drivers/infiniband/hw/mlx4/cq.c
> > index 5a219a2..4366811 100644
> > --- a/drivers/infiniband/hw/mlx4/cq.c
> > +++ b/drivers/infiniband/hw/mlx4/cq.c
> > @@ -755,6 +755,13 @@ int mlx4_ib_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
> >                    to_mdev(ibcq->device)->uar_map,
> >                    MLX4_GET_DOORBELL_LOCK(&to_mdev(ibcq->device)->uar_lock));
> >
> > +       if (flags & IB_CQ_REPORT_MISSED_EVENTS) {
> > +               struct mlx4_cqe *cqe;
> > +               cqe = next_cqe_sw(to_mcq(ibcq));
> > +               if (cqe)
> > +                       return 1;
> > +       }
> > +
> >        return 0;
> >  }
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib/mlx4: add IB_CQ_REPORT_MISSED_EVENTS support

[Or Gerlitz]

Eli Cohen wrote:
> returning 1 means that you must poll the CQ to avoid a race condition
> which is not true for mlx4. 

makes sense, thanks for clarifying that.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html