On Fri, Jan 18, 2019 at 01:12:36PM -0800, Dennis Dalessandro wrote:
> @@ -480,7 +533,7 @@ int rvt_resize_cq(struct ib_cq *ibcq, int cqe, struct ib_udata *udata)
> int rvt_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *entry)
> {
> struct rvt_cq *cq = ibcq_to_rvtcq(ibcq);
> - struct rvt_cq_wc *wc;
> + struct rvt_k_cq_wc *wc;
> unsigned long flags;
> int npolled;
> u32 tail;
> @@ -491,7 +544,7 @@ int rvt_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *entry)
>
> spin_lock_irqsave(&cq->lock, flags);
>
> - wc = cq->queue;
> + wc = cq->kqueue;
Where are the READ_ONCE/WRITE_ONCE I said was neeed?
> tail = wc->tail;
> if (tail > (u32)cq->ibcq.cqe)
> tail = (u32)cq->ibcq.cqe;
> diff --git a/include/rdma/rdmavt_cq.h b/include/rdma/rdmavt_cq.h
> index 75dc65c..9f25be0 100644
> +++ b/include/rdma/rdmavt_cq.h
> @@ -59,20 +59,17 @@
> * notifications are armed.
> */
> #define RVT_CQ_NONE (IB_CQ_NEXT_COMP + 1)
> +#include <rdma/rvt-abi.h>
>
> /*
> * This structure is used to contain the head pointer, tail pointer,
> * and completion queue entries as a single memory allocation so
> * it can be mmap'ed into user space.
> */
> -struct rvt_cq_wc {
> +struct rvt_k_cq_wc {
Eh? Why keeping two identical structures with the same comment? Are
they being used for the same thing? Why?
> u32 head; /* index of next entry to fill */
> u32 tail; /* index of next ib_poll_cq() entry */
> - union {
> - /* these are actually size ibcq.cqe + 1 */
> - struct ib_uverbs_wc uqueue[0];
> - struct ib_wc kqueue[0];
> - };
> + struct ib_wc kqueue[0];
> };
>
> /*
> @@ -88,6 +85,7 @@ struct rvt_cq {
> struct rvt_dev_info *rdi;
> struct rvt_cq_wc *queue;
> struct rvt_mmap_info *ip;
> + struct rvt_k_cq_wc *kqueue;
> };
>
> static inline struct rvt_cq *ibcq_to_rvtcq(struct ib_cq *ibcq)
> diff --git a/include/uapi/rdma/rvt-abi.h b/include/uapi/rdma/rvt-abi.h
> new file mode 100644
> index 0000000..c9ad6fa
> +++ b/include/uapi/rdma/rvt-abi.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
> +
> +/*
> + * This file contains defines, structures, etc. that are used
> + * to communicate between kernel and user code.
> + */
> +
> +#ifndef RVT_ABI_USER_H
> +#define RVT_ABI_USER_H
> +
> +#include <linux/types.h>
> +#include <rdma/ib_user_verbs.h>
> +#ifndef RDMA_ATOMIC_UAPI
> +#define RDMA_ATOMIC_UAPI(_type, _name) _type _name
> +#endif
> +/*
> + * This structure is used to contain the head pointer, tail pointer,
> + * and completion queue entries as a single memory allocation so
> + * it can be mmap'ed into user space.
> + */
> +struct rvt_cq_wc {
> + /* index of next entry to fill */
> + RDMA_ATOMIC_UAPI(u32, head);
> + /* index of next ib_poll_cq() entry */
> + RDMA_ATOMIC_UAPI(u32, tail);
__u32 surely? Did this compile in rdma-core?
Jason
