* [PATCH AUTOSEL 5.1 80/95] IB/hfi1: Wakeup QPs orphaned on wait list after flush
From: Sasha Levin @ 2019-06-27 0:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mike Marciniszyn, Kaike Wan, Dennis Dalessandro, Doug Ledford,
Sasha Levin, linux-rdma
In-Reply-To: <20190627003021.19867-1-sashal@kernel.org>
From: Mike Marciniszyn <mike.marciniszyn@intel.com>
[ Upstream commit f972775b1cc0441ae22c9f8d06dd16b118463632 ]
Once an SDMA engine is taken down due to a link failure, any waiting QPs
that do not have outstanding descriptors in the ring will stay
on the dmawait list as long as the port is down.
Since there is no timer running, they will stay there for a long time.
The fix is to wake up all iowaits linked to dmawait. The send engine
will build and post packets that get flushed back.
Fixes: 7724105686e7 ("IB/hfi1: add driver files")
Reviewed-by: Kaike Wan <kaike.wan@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/hw/hfi1/sdma.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/infiniband/hw/hfi1/sdma.c b/drivers/infiniband/hw/hfi1/sdma.c
index b0110728f541..1cde1b8f0c8b 100644
--- a/drivers/infiniband/hw/hfi1/sdma.c
+++ b/drivers/infiniband/hw/hfi1/sdma.c
@@ -405,6 +405,7 @@ static void sdma_flush(struct sdma_engine *sde)
struct sdma_txreq *txp, *txp_next;
LIST_HEAD(flushlist);
unsigned long flags;
+ uint seq;
/* flush from head to tail */
sdma_flush_descq(sde);
@@ -418,6 +419,22 @@ static void sdma_flush(struct sdma_engine *sde)
/* flush from flush list */
list_for_each_entry_safe(txp, txp_next, &flushlist, list)
complete_tx(sde, txp, SDMA_TXREQ_S_ABORTED);
+ /* wakeup QPs orphaned on the dmawait list */
+ do {
+ struct iowait *w, *nw;
+
+ seq = read_seqbegin(&sde->waitlock);
+ if (!list_empty(&sde->dmawait)) {
+ write_seqlock(&sde->waitlock);
+ list_for_each_entry_safe(w, nw, &sde->dmawait, list) {
+ if (w->wakeup) {
+ w->wakeup(w, SDMA_AVAIL_REASON);
+ list_del_init(&w->list);
+ }
+ }
+ write_sequnlock(&sde->waitlock);
+ }
+ } while (read_seqretry(&sde->waitlock, seq));
}
/*
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.1 79/95] IB/hfi1: Use aborts to trigger RC throttling
From: Sasha Levin @ 2019-06-27 0:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mike Marciniszyn, Kaike Wan, Dennis Dalessandro, Doug Ledford,
Sasha Levin, linux-rdma
In-Reply-To: <20190627003021.19867-1-sashal@kernel.org>
From: Mike Marciniszyn <mike.marciniszyn@intel.com>
[ Upstream commit 4bb02e9572af1383038d83ad196d7166c515f2ee ]
SDMA and pio flushes will cause a lot of packets to be transmitted
after a link has gone down, using a lot of CPU to retransmit
packets.
Fix for RC QPs by recognizing the flush status and:
- Forcing a timer start
- Putting the QP into a "send one" mode
Fixes: 7724105686e7 ("IB/hfi1: add driver files")
Reviewed-by: Kaike Wan <kaike.wan@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/hw/hfi1/rc.c | 30 ++++++++++++++++++++++++++++++
drivers/infiniband/hw/hfi1/verbs.c | 10 ++++++----
drivers/infiniband/hw/hfi1/verbs.h | 1 +
3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index 82f101878e33..24cbac277bf0 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -1703,6 +1703,36 @@ static void reset_sending_psn(struct rvt_qp *qp, u32 psn)
}
}
+/**
+ * hfi1_rc_verbs_aborted - handle abort status
+ * @qp: the QP
+ * @opah: the opa header
+ *
+ * This code modifies both ACK bit in BTH[2]
+ * and the s_flags to go into send one mode.
+ *
+ * This serves to throttle the send engine to only
+ * send a single packet in the likely case the
+ * a link has gone down.
+ */
+void hfi1_rc_verbs_aborted(struct rvt_qp *qp, struct hfi1_opa_header *opah)
+{
+ struct ib_other_headers *ohdr = hfi1_get_rc_ohdr(opah);
+ u8 opcode = ib_bth_get_opcode(ohdr);
+ u32 psn;
+
+ /* ignore responses */
+ if ((opcode >= OP(RDMA_READ_RESPONSE_FIRST) &&
+ opcode <= OP(ATOMIC_ACKNOWLEDGE)) ||
+ opcode == TID_OP(READ_RESP) ||
+ opcode == TID_OP(WRITE_RESP))
+ return;
+
+ psn = ib_bth_get_psn(ohdr) | IB_BTH_REQ_ACK;
+ ohdr->bth[2] = cpu_to_be32(psn);
+ qp->s_flags |= RVT_S_SEND_ONE;
+}
+
/*
* This should be called with the QP s_lock held and interrupts disabled.
*/
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index 55a56b3d7f83..8d64972c6226 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -638,6 +638,8 @@ static void verbs_sdma_complete(
struct hfi1_opa_header *hdr;
hdr = &tx->phdr.hdr;
+ if (unlikely(status == SDMA_TXREQ_S_ABORTED))
+ hfi1_rc_verbs_aborted(qp, hdr);
hfi1_rc_send_complete(qp, hdr);
}
spin_unlock(&qp->s_lock);
@@ -1095,15 +1097,15 @@ int hfi1_verbs_send_pio(struct rvt_qp *qp, struct hfi1_pkt_state *ps,
&ps->s_txreq->phdr.hdr, ib_is_sc5(sc5));
pio_bail:
+ spin_lock_irqsave(&qp->s_lock, flags);
if (qp->s_wqe) {
- spin_lock_irqsave(&qp->s_lock, flags);
rvt_send_complete(qp, qp->s_wqe, wc_status);
- spin_unlock_irqrestore(&qp->s_lock, flags);
} else if (qp->ibqp.qp_type == IB_QPT_RC) {
- spin_lock_irqsave(&qp->s_lock, flags);
+ if (unlikely(wc_status == IB_WC_GENERAL_ERR))
+ hfi1_rc_verbs_aborted(qp, &ps->s_txreq->phdr.hdr);
hfi1_rc_send_complete(qp, &ps->s_txreq->phdr.hdr);
- spin_unlock_irqrestore(&qp->s_lock, flags);
}
+ spin_unlock_irqrestore(&qp->s_lock, flags);
ret = 0;
diff --git a/drivers/infiniband/hw/hfi1/verbs.h b/drivers/infiniband/hw/hfi1/verbs.h
index 62ace0b2d17a..1714c0f6475d 100644
--- a/drivers/infiniband/hw/hfi1/verbs.h
+++ b/drivers/infiniband/hw/hfi1/verbs.h
@@ -415,6 +415,7 @@ void hfi1_rc_hdrerr(
u8 ah_to_sc(struct ib_device *ibdev, struct rdma_ah_attr *ah_attr);
+void hfi1_rc_verbs_aborted(struct rvt_qp *qp, struct hfi1_opa_header *opah);
void hfi1_rc_send_complete(struct rvt_qp *qp, struct hfi1_opa_header *opah);
void hfi1_ud_rcv(struct hfi1_packet *packet);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.1 78/95] IB/hfi1: Create inline to get extended headers
From: Sasha Levin @ 2019-06-27 0:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mike Marciniszyn, Dennis Dalessandro, Doug Ledford, Sasha Levin,
linux-rdma
In-Reply-To: <20190627003021.19867-1-sashal@kernel.org>
From: Mike Marciniszyn <mike.marciniszyn@intel.com>
[ Upstream commit 9755f72496664eec70bc804104118b5797b6bf63 ]
This paves the way for another patch that reacts to a
flush sdma completion for RC.
Fixes: 81cd3891f021 ("IB/hfi1: Add support for 16B Management Packets")
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/hw/hfi1/hfi.h | 31 +++++++++++++++++++++++++++++++
drivers/infiniband/hw/hfi1/rc.c | 21 +--------------------
2 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/hfi.h b/drivers/infiniband/hw/hfi1/hfi.h
index 048b5d73ba39..d85b16a3aaaf 100644
--- a/drivers/infiniband/hw/hfi1/hfi.h
+++ b/drivers/infiniband/hw/hfi1/hfi.h
@@ -539,6 +539,37 @@ static inline void hfi1_16B_set_qpn(struct opa_16b_mgmt *mgmt,
mgmt->src_qpn = cpu_to_be32(src_qp & OPA_16B_MGMT_QPN_MASK);
}
+/**
+ * hfi1_get_rc_ohdr - get extended header
+ * @opah - the opaheader
+ */
+static inline struct ib_other_headers *
+hfi1_get_rc_ohdr(struct hfi1_opa_header *opah)
+{
+ struct ib_other_headers *ohdr;
+ struct ib_header *hdr = NULL;
+ struct hfi1_16b_header *hdr_16b = NULL;
+
+ /* Find out where the BTH is */
+ if (opah->hdr_type == HFI1_PKT_TYPE_9B) {
+ hdr = &opah->ibh;
+ if (ib_get_lnh(hdr) == HFI1_LRH_BTH)
+ ohdr = &hdr->u.oth;
+ else
+ ohdr = &hdr->u.l.oth;
+ } else {
+ u8 l4;
+
+ hdr_16b = &opah->opah;
+ l4 = hfi1_16B_get_l4(hdr_16b);
+ if (l4 == OPA_16B_L4_IB_LOCAL)
+ ohdr = &hdr_16b->u.oth;
+ else
+ ohdr = &hdr_16b->u.l.oth;
+ }
+ return ohdr;
+}
+
struct rvt_sge_state;
/*
diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index 5991211d72bd..82f101878e33 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -1711,8 +1711,6 @@ void hfi1_rc_send_complete(struct rvt_qp *qp, struct hfi1_opa_header *opah)
struct ib_other_headers *ohdr;
struct hfi1_qp_priv *priv = qp->priv;
struct rvt_swqe *wqe;
- struct ib_header *hdr = NULL;
- struct hfi1_16b_header *hdr_16b = NULL;
u32 opcode, head, tail;
u32 psn;
struct tid_rdma_request *req;
@@ -1721,24 +1719,7 @@ void hfi1_rc_send_complete(struct rvt_qp *qp, struct hfi1_opa_header *opah)
if (!(ib_rvt_state_ops[qp->state] & RVT_SEND_OR_FLUSH_OR_RECV_OK))
return;
- /* Find out where the BTH is */
- if (priv->hdr_type == HFI1_PKT_TYPE_9B) {
- hdr = &opah->ibh;
- if (ib_get_lnh(hdr) == HFI1_LRH_BTH)
- ohdr = &hdr->u.oth;
- else
- ohdr = &hdr->u.l.oth;
- } else {
- u8 l4;
-
- hdr_16b = &opah->opah;
- l4 = hfi1_16B_get_l4(hdr_16b);
- if (l4 == OPA_16B_L4_IB_LOCAL)
- ohdr = &hdr_16b->u.oth;
- else
- ohdr = &hdr_16b->u.l.oth;
- }
-
+ ohdr = hfi1_get_rc_ohdr(opah);
opcode = ib_bth_get_opcode(ohdr);
if ((opcode >= OP(RDMA_READ_RESPONSE_FIRST) &&
opcode <= OP(ATOMIC_ACKNOWLEDGE)) ||
--
2.20.1
^ permalink raw reply related
* Re: [PATCH mlx5-next v1 02/12] net/mlx5: Use event mask based on device capabilities
From: Saeed Mahameed @ 2019-06-27 0:23 UTC (permalink / raw)
To: Jason Gunthorpe, leon@kernel.org, dledford@redhat.com
Cc: Yishai Hadas, netdev@vger.kernel.org, Leon Romanovsky,
linux-rdma@vger.kernel.org
In-Reply-To: <20190618171540.11729-3-leon@kernel.org>
On Tue, 2019-06-18 at 20:15 +0300, Leon Romanovsky wrote:
> From: Yishai Hadas <yishaih@mellanox.com>
>
> Use the reported device capabilities for the supported user events
> (i.e.
> affiliated and un-affiliated) to set the EQ mask.
>
> As the event mask can be up to 256 defined by 4 entries of u64 change
> the applicable code to work accordingly.
>
> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/hw/mlx5/odp.c | 3 +-
> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 45 ++++++++++++++++
> ----
> drivers/net/ethernet/mellanox/mlx5/core/fw.c | 6 +++
> include/linux/mlx5/device.h | 6 ++-
> include/linux/mlx5/eq.h | 4 +-
> include/linux/mlx5/mlx5_ifc.h | 13 ++++--
> 6 files changed, 63 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/odp.c
> b/drivers/infiniband/hw/mlx5/odp.c
> index 600fe23e2eae..a6740ec308ed 100644
> --- a/drivers/infiniband/hw/mlx5/odp.c
> +++ b/drivers/infiniband/hw/mlx5/odp.c
> @@ -1559,10 +1559,11 @@ mlx5_ib_create_pf_eq(struct mlx5_ib_dev *dev,
> struct mlx5_ib_pf_eq *eq)
> eq->irq_nb.notifier_call = mlx5_ib_eq_pf_int;
> param = (struct mlx5_eq_param) {
> .irq_index = 0,
> - .mask = 1 << MLX5_EVENT_TYPE_PAGE_FAULT,
> .nent = MLX5_IB_NUM_PF_EQE,
> };
> eq->core = mlx5_eq_create_generic(dev->mdev, ¶m);
> +
> + param.mask[0] = 1ull << MLX5_EVENT_TYPE_PAGE_FAULT;
As Yishai already pointer out, there is a regression here,
the line above was merged in the wrong order, mask should be setup
before calling mlx5_eq_create_generic.
I will expect V3.
> if (IS_ERR(eq->core)) {
> err = PTR_ERR(eq->core);
> goto err_wq;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
> index 8000d2a4a7e2..9d07add38940 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
> @@ -256,6 +256,7 @@ create_map_eq(struct mlx5_core_dev *dev, struct
> mlx5_eq *eq,
> int inlen;
> u32 *in;
> int err;
> + int i;
>
> /* Init CQ table */
> memset(cq_table, 0, sizeof(*cq_table));
> @@ -283,10 +284,12 @@ create_map_eq(struct mlx5_core_dev *dev, struct
> mlx5_eq *eq,
> mlx5_fill_page_array(&eq->buf, pas);
>
> MLX5_SET(create_eq_in, in, opcode, MLX5_CMD_OP_CREATE_EQ);
> - if (!param->mask && MLX5_CAP_GEN(dev, log_max_uctx))
> + if (!param->mask[0] && MLX5_CAP_GEN(dev, log_max_uctx))
> MLX5_SET(create_eq_in, in, uid,
> MLX5_SHARED_RESOURCE_UID);
>
> - MLX5_SET64(create_eq_in, in, event_bitmask, param->mask);
> + for (i = 0; i < 4; i++)
> + MLX5_ARRAY_SET64(create_eq_in, in, event_bitmask, i,
> + param->mask[i]);
>
> eqc = MLX5_ADDR_OF(create_eq_in, in, eq_context_entry);
> MLX5_SET(eqc, eqc, log_eq_size, ilog2(eq->nent));
> @@ -507,10 +510,32 @@ static int cq_err_event_notifier(struct
> notifier_block *nb,
> return NOTIFY_OK;
> }
>
> -static u64 gather_async_events_mask(struct mlx5_core_dev *dev)
> +static void gather_async_events_from_cap(struct mlx5_core_dev *dev,
> + u64 mask[4])
> +{
> + __be64 *user_unaffiliated_events;
> + __be64 *user_affiliated_events;
> + int i;
> +
> + user_affiliated_events =
> + MLX5_CAP_DEV_EVENT(dev, user_affiliated_events);
> + user_unaffiliated_events =
> + MLX5_CAP_DEV_EVENT(dev, user_unaffiliated_events);
> +
> + for (i = 0; i < 4; i++)
> + mask[i] = be64_to_cpu(user_affiliated_events[i] |
> + user_unaffiliated_events[i]);
> +}
> +
> +static void gather_async_events_mask(struct mlx5_core_dev *dev, u64
> mask[4])
> {
> u64 async_event_mask = MLX5_ASYNC_EVENT_MASK;
>
> + if (MLX5_CAP_GEN(dev, event_cap)) {
> + gather_async_events_from_cap(dev, mask);
> + return;
> + }
> +
> if (MLX5_VPORT_MANAGER(dev))
> async_event_mask |= (1ull <<
> MLX5_EVENT_TYPE_NIC_VPORT_CHANGE);
>
> @@ -544,7 +569,7 @@ static u64 gather_async_events_mask(struct
> mlx5_core_dev *dev)
> async_event_mask |=
> (1ull <<
> MLX5_EVENT_TYPE_ESW_FUNCTIONS_CHANGED);
>
> - return async_event_mask;
> + mask[0] = async_event_mask;
> }
>
> static int create_async_eqs(struct mlx5_core_dev *dev)
> @@ -559,9 +584,11 @@ static int create_async_eqs(struct mlx5_core_dev
> *dev)
> table->cmd_eq.irq_nb.notifier_call = mlx5_eq_async_int;
> param = (struct mlx5_eq_param) {
> .irq_index = 0,
> - .mask = 1ull << MLX5_EVENT_TYPE_CMD,
> + .mask = {1ull << MLX5_EVENT_TYPE_CMD},
> .nent = MLX5_NUM_CMD_EQE,
> };
> +
> + param.mask[0] = 1ull << MLX5_EVENT_TYPE_CMD;
No need to setup the mask twice,
here and everywhere in this patch, pick one approach to set it.
> err = create_async_eq(dev, &table->cmd_eq.core, ¶m);
> if (err) {
> mlx5_core_warn(dev, "failed to create cmd EQ %d\n",
> err);
> @@ -577,9 +604,9 @@ static int create_async_eqs(struct mlx5_core_dev
> *dev)
> table->async_eq.irq_nb.notifier_call = mlx5_eq_async_int;
> param = (struct mlx5_eq_param) {
> .irq_index = 0,
> - .mask = gather_async_events_mask(dev),
> .nent = MLX5_NUM_ASYNC_EQE,
> };
> + gather_async_events_mask(dev, param.mask);
> err = create_async_eq(dev, &table->async_eq.core, ¶m);
> if (err) {
> mlx5_core_warn(dev, "failed to create async EQ %d\n",
> err);
> @@ -595,9 +622,11 @@ static int create_async_eqs(struct mlx5_core_dev
> *dev)
> table->pages_eq.irq_nb.notifier_call = mlx5_eq_async_int;
> param = (struct mlx5_eq_param) {
> .irq_index = 0,
> - .mask = 1 << MLX5_EVENT_TYPE_PAGE_REQUEST,
> + .mask = {1 << MLX5_EVENT_TYPE_PAGE_REQUEST},
> .nent = /* TODO: sriov max_vf + */ 1,
> };
> +
> + param.mask[0] = 1ull << MLX5_EVENT_TYPE_PAGE_REQUEST;
> err = create_async_eq(dev, &table->pages_eq.core, ¶m);
> if (err) {
> mlx5_core_warn(dev, "failed to create pages EQ %d\n",
> err);
> @@ -789,7 +818,7 @@ static int create_comp_eqs(struct mlx5_core_dev
> *dev)
> eq->irq_nb.notifier_call = mlx5_eq_comp_int;
> param = (struct mlx5_eq_param) {
> .irq_index = vecidx,
> - .mask = 0,
> + .mask = {0},
> .nent = nent,
> };
> err = create_map_eq(dev, &eq->core, ¶m);
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw.c
> b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
> index 1ab6f7e3bec6..05367f15c3a7 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fw.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
> @@ -202,6 +202,12 @@ int mlx5_query_hca_caps(struct mlx5_core_dev
> *dev)
> return err;
> }
>
> + if (MLX5_CAP_GEN(dev, event_cap)) {
> + err = mlx5_core_get_caps(dev, MLX5_CAP_DEV_EVENT);
> + if (err)
> + return err;
> + }
> +
> return 0;
> }
>
> diff --git a/include/linux/mlx5/device.h
> b/include/linux/mlx5/device.h
> index 5e760067ac41..0d1abe097627 100644
> --- a/include/linux/mlx5/device.h
> +++ b/include/linux/mlx5/device.h
> @@ -351,7 +351,7 @@ enum mlx5_event {
>
> MLX5_EVENT_TYPE_DEVICE_TRACER = 0x26,
>
> - MLX5_EVENT_TYPE_MAX =
> MLX5_EVENT_TYPE_DEVICE_TRACER + 1,
> + MLX5_EVENT_TYPE_MAX = 0x100,
> };
>
> enum {
> @@ -1077,6 +1077,7 @@ enum mlx5_cap_type {
> MLX5_CAP_DEBUG,
> MLX5_CAP_RESERVED_14,
> MLX5_CAP_DEV_MEM,
> + MLX5_CAP_DEV_EVENT = 0x14,
> /* NUM OF CAP Types */
> MLX5_CAP_NUM
> };
> @@ -1255,6 +1256,9 @@ enum mlx5_qcam_feature_groups {
> #define MLX5_CAP64_DEV_MEM(mdev, cap)\
> MLX5_GET64(device_mem_cap, mdev-
> >caps.hca_cur[MLX5_CAP_DEV_MEM], cap)
>
> +#define MLX5_CAP_DEV_EVENT(mdev, cap)\
> + MLX5_ADDR_OF(device_event_cap, (mdev)-
> >caps.hca_cur[MLX5_CAP_DEV_EVENT], cap)
> +
> enum {
> MLX5_CMD_STAT_OK = 0x0,
> MLX5_CMD_STAT_INT_ERR = 0x1,
> diff --git a/include/linux/mlx5/eq.h b/include/linux/mlx5/eq.h
> index 70e16dcfb4c4..202df2e5fe8c 100644
> --- a/include/linux/mlx5/eq.h
> +++ b/include/linux/mlx5/eq.h
> @@ -15,7 +15,9 @@ struct mlx5_core_dev;
> struct mlx5_eq_param {
> u8 irq_index;
> int nent;
> - u64 mask;
> + u64 mask[4];
> + void *context;
> + irq_handler_t handler;
> };
>
> struct mlx5_eq *
> diff --git a/include/linux/mlx5/mlx5_ifc.h
> b/include/linux/mlx5/mlx5_ifc.h
> index 16348528fef6..3ef716c054c2 100644
> --- a/include/linux/mlx5/mlx5_ifc.h
> +++ b/include/linux/mlx5/mlx5_ifc.h
> @@ -823,6 +823,12 @@ struct mlx5_ifc_device_mem_cap_bits {
> u8 reserved_at_180[0x680];
> };
>
> +struct mlx5_ifc_device_event_cap_bits {
> + u8 user_affiliated_events[4][0x40];
> +
> + u8 user_unaffiliated_events[4][0x40];
> +};
> +
> enum {
> MLX5_ATOMIC_CAPS_ATOMIC_SIZE_QP_1_BYTE = 0x0,
> MLX5_ATOMIC_CAPS_ATOMIC_SIZE_QP_2_BYTES = 0x2,
> @@ -980,7 +986,8 @@ struct mlx5_ifc_cmd_hca_cap_bits {
>
> u8 log_max_srq_sz[0x8];
> u8 log_max_qp_sz[0x8];
> - u8 reserved_at_90[0x8];
> + u8 event_cap[0x1];
> + u8 reserved_at_91[0x7];
> u8 prio_tag_required[0x1];
> u8 reserved_at_99[0x2];
> u8 log_max_qp[0x5];
> @@ -7364,9 +7371,9 @@ struct mlx5_ifc_create_eq_in_bits {
>
> u8 reserved_at_280[0x40];
>
> - u8 event_bitmask[0x40];
> + u8 event_bitmask[4][0x40];
>
> - u8 reserved_at_300[0x580];
> + u8 reserved_at_3c0[0x4c0];
>
> u8 pas[0][0x40];
> };
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Logan Gunthorpe @ 2019-06-26 21:18 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Christoph Hellwig, linux-kernel, linux-block, linux-nvme,
linux-pci, linux-rdma, Jens Axboe, Bjorn Helgaas, Dan Williams,
Sagi Grimberg, Keith Busch, Stephen Bates
In-Reply-To: <20190626210018.GB6392@ziepe.ca>
On 2019-06-26 3:00 p.m., Jason Gunthorpe wrote:
> On Wed, Jun 26, 2019 at 02:45:38PM -0600, Logan Gunthorpe wrote:
>>
>>
>> On 2019-06-26 2:21 p.m., Jason Gunthorpe wrote:
>>> On Wed, Jun 26, 2019 at 12:31:08PM -0600, Logan Gunthorpe wrote:
>>>>> we have a hole behind len where we could store flag. Preferably
>>>>> optionally based on a P2P or other magic memory types config
>>>>> option so that 32-bit systems with 32-bit phys_addr_t actually
>>>>> benefit from the smaller and better packing structure.
>>>>
>>>> That seems sensible. The one thing that's unclear though is how to get
>>>> the PCI Bus address when appropriate. Can we pass that in instead of the
>>>> phys_addr with an appropriate flag? Or will we need to pass the actual
>>>> physical address and then, at the map step, the driver has to some how
>>>> lookup the PCI device to figure out the bus offset?
>>>
>>> I agree with CH, if we go down this path it is a layering violation
>>> for the thing injecting bio's into the block stack to know what struct
>>> device they egress&dma map on just to be able to do the dma_map up
>>> front.
>>
>> Not sure I agree with this statement. The p2pdma code already *must*
>> know and access the pci_dev of the dma device ahead of when it submits
>> the IO to know if it's valid to allocate and use P2P memory at all.
>
> I don't think we should make drives do that. What if it got CMB memory
> on some other device?
Huh? A driver submitting P2P requests finds appropriate memory to use
based on the DMA device that will be doing the mapping. It *has* to. It
doesn't necessarily have control over which P2P provider it might find
(ie. it may get CMB memory from a random NVMe device), but it easily
knows the NVMe device it got the CMB memory for. Look at the existing
code in the nvme target.
>>> For instance we could use a small hash table of the upper phys addr
>>> bits, or an interval tree, to do the lookup.
>>
>> Yes, if we're going to take a hard stance on this. But using an interval
>> tree (or similar) is a lot more work for the CPU to figure out these
>> mappings that may not be strictly necessary if we could just pass better
>> information down from the submitting driver to the mapping driver.
>
> Right, this is coming down to an optimization argument. I think there
> are very few cases (Basically yours) where the caller will know this
> info, so we need to support the other cases anyhow.
I disagree. I think it has to be a common pattern. A driver doing a P2P
transaction *must* find some device to obtain memory from (or it may be
itself) and check if it is compatible with the device that's going to
be mapping the memory or vice versa. So no matter what we do, a driver
submitting P2P requests must have access to both the PCI device that's
going to be mapping the memory and the device that's providing the memory.
> I think with some simple caching this will become negligible for cases
> you care about
Well *maybe* it will be negligible performance wise, but it's also a lot
more complicated, code wise. Tree lookups will always be a lot more
expensive than just checking a flag.
Logan
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Jason Gunthorpe @ 2019-06-26 21:00 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: Christoph Hellwig, linux-kernel, linux-block, linux-nvme,
linux-pci, linux-rdma, Jens Axboe, Bjorn Helgaas, Dan Williams,
Sagi Grimberg, Keith Busch, Stephen Bates
In-Reply-To: <8a0a08c3-a537-bff6-0852-a5f337a70688@deltatee.com>
On Wed, Jun 26, 2019 at 02:45:38PM -0600, Logan Gunthorpe wrote:
>
>
> On 2019-06-26 2:21 p.m., Jason Gunthorpe wrote:
> > On Wed, Jun 26, 2019 at 12:31:08PM -0600, Logan Gunthorpe wrote:
> >>> we have a hole behind len where we could store flag. Preferably
> >>> optionally based on a P2P or other magic memory types config
> >>> option so that 32-bit systems with 32-bit phys_addr_t actually
> >>> benefit from the smaller and better packing structure.
> >>
> >> That seems sensible. The one thing that's unclear though is how to get
> >> the PCI Bus address when appropriate. Can we pass that in instead of the
> >> phys_addr with an appropriate flag? Or will we need to pass the actual
> >> physical address and then, at the map step, the driver has to some how
> >> lookup the PCI device to figure out the bus offset?
> >
> > I agree with CH, if we go down this path it is a layering violation
> > for the thing injecting bio's into the block stack to know what struct
> > device they egress&dma map on just to be able to do the dma_map up
> > front.
>
> Not sure I agree with this statement. The p2pdma code already *must*
> know and access the pci_dev of the dma device ahead of when it submits
> the IO to know if it's valid to allocate and use P2P memory at all.
I don't think we should make drives do that. What if it got CMB memory
on some other device?
> > For instance we could use a small hash table of the upper phys addr
> > bits, or an interval tree, to do the lookup.
>
> Yes, if we're going to take a hard stance on this. But using an interval
> tree (or similar) is a lot more work for the CPU to figure out these
> mappings that may not be strictly necessary if we could just pass better
> information down from the submitting driver to the mapping driver.
Right, this is coming down to an optimization argument. I think there
are very few cases (Basically yours) where the caller will know this
info, so we need to support the other cases anyhow.
I think with some simple caching this will become negligible for cases
you care about
Jason
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Logan Gunthorpe @ 2019-06-26 20:55 UTC (permalink / raw)
To: Dan Williams, Jason Gunthorpe
Cc: Christoph Hellwig, Linux Kernel Mailing List, linux-block,
linux-nvme, linux-pci, linux-rdma, Jens Axboe, Bjorn Helgaas,
Sagi Grimberg, Keith Busch, Stephen Bates
In-Reply-To: <CAPcyv4hCNoMeFyOE588=kuNUXaPS-rzaXnF2cN2TFejso1SGRw@mail.gmail.com>
On 2019-06-26 2:39 p.m., Dan Williams wrote:
> On Wed, Jun 26, 2019 at 1:21 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>>
>> On Wed, Jun 26, 2019 at 12:31:08PM -0600, Logan Gunthorpe wrote:
>>>> we have a hole behind len where we could store flag. Preferably
>>>> optionally based on a P2P or other magic memory types config
>>>> option so that 32-bit systems with 32-bit phys_addr_t actually
>>>> benefit from the smaller and better packing structure.
>>>
>>> That seems sensible. The one thing that's unclear though is how to get
>>> the PCI Bus address when appropriate. Can we pass that in instead of the
>>> phys_addr with an appropriate flag? Or will we need to pass the actual
>>> physical address and then, at the map step, the driver has to some how
>>> lookup the PCI device to figure out the bus offset?
>>
>> I agree with CH, if we go down this path it is a layering violation
>> for the thing injecting bio's into the block stack to know what struct
>> device they egress&dma map on just to be able to do the dma_map up
>> front.
>>
>> So we must be able to go from this new phys_addr_t&flags to some BAR
>> information during dma_map.
>>
>> For instance we could use a small hash table of the upper phys addr
>> bits, or an interval tree, to do the lookup.
>
> Hmm, that sounds like dev_pagemap without the pages.
Yup, that's why I'd like to avoid it, but IMO it would still be an
improvement to use a interval tree over struct pages because without
struct page we just have a range and a length and it's relatively easy
to check that the whole range belongs to a specific pci_dev. To be
correct with the struct page approach we really have to loop through all
pages to ensure they all belong to the same pci_dev which is a big pain.
> There's already no requirement that dev_pagemap point to real /
> present pages (DEVICE_PRIVATE) seems a straightforward extension to
> use it for helping coordinate phys_addr_t in 'struct bio'. Then
> Logan's future plans to let userspace coordinate p2p operations could
> build on PTE_DEVMAP.
Well I think the biggest difficulty with struct page for user space is
dealing with cases when the struct pages of different types get mixed
together (or even struct pages that are all P2P pages but from different
PCI devices). We'd have to go through each page and ensure that each
type gets it's own bio_vec with appropriate flags.
Though really, the whole mixed IO from userspace poses a bunch of
problems. I'd prefer to just be able to say that a single IO can be all
or nothing P2P memory from a single device.
Logan
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Jason Gunthorpe @ 2019-06-26 20:54 UTC (permalink / raw)
To: Dan Williams
Cc: Logan Gunthorpe, Christoph Hellwig, Linux Kernel Mailing List,
linux-block, linux-nvme, linux-pci, linux-rdma, Jens Axboe,
Bjorn Helgaas, Sagi Grimberg, Keith Busch, Stephen Bates
In-Reply-To: <CAPcyv4hCNoMeFyOE588=kuNUXaPS-rzaXnF2cN2TFejso1SGRw@mail.gmail.com>
On Wed, Jun 26, 2019 at 01:39:01PM -0700, Dan Williams wrote:
> Hmm, that sounds like dev_pagemap without the pages.
Yes, and other page related overhead. Maybe both ideas can exist in
the pagemap code?
All that is needed here is to map a bar phys_addr_t to some 'bar info'
that helps the mapping.
Jason
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Logan Gunthorpe @ 2019-06-26 20:45 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Christoph Hellwig, linux-kernel, linux-block, linux-nvme,
linux-pci, linux-rdma, Jens Axboe, Bjorn Helgaas, Dan Williams,
Sagi Grimberg, Keith Busch, Stephen Bates
In-Reply-To: <20190626202107.GA5850@ziepe.ca>
On 2019-06-26 2:21 p.m., Jason Gunthorpe wrote:
> On Wed, Jun 26, 2019 at 12:31:08PM -0600, Logan Gunthorpe wrote:
>>> we have a hole behind len where we could store flag. Preferably
>>> optionally based on a P2P or other magic memory types config
>>> option so that 32-bit systems with 32-bit phys_addr_t actually
>>> benefit from the smaller and better packing structure.
>>
>> That seems sensible. The one thing that's unclear though is how to get
>> the PCI Bus address when appropriate. Can we pass that in instead of the
>> phys_addr with an appropriate flag? Or will we need to pass the actual
>> physical address and then, at the map step, the driver has to some how
>> lookup the PCI device to figure out the bus offset?
>
> I agree with CH, if we go down this path it is a layering violation
> for the thing injecting bio's into the block stack to know what struct
> device they egress&dma map on just to be able to do the dma_map up
> front.
Not sure I agree with this statement. The p2pdma code already *must*
know and access the pci_dev of the dma device ahead of when it submits
the IO to know if it's valid to allocate and use P2P memory at all. This
is why the submitting driver has a lot of the information needed to map
this memory that the mapping driver does not.
> So we must be able to go from this new phys_addr_t&flags to some BAR
> information during dma_map.
> For instance we could use a small hash table of the upper phys addr
> bits, or an interval tree, to do the lookup.
Yes, if we're going to take a hard stance on this. But using an interval
tree (or similar) is a lot more work for the CPU to figure out these
mappings that may not be strictly necessary if we could just pass better
information down from the submitting driver to the mapping driver.
> The bar info would give the exporting struct device and any other info
> we need to make the iommu mapping.
Well, the IOMMU mapping is the normal thing the mapping driver will
always do. We'd really just need the submitting driver to, when
appropriate, inform the mapping driver that this is a pci bus address
and not to call dma_map_xxx(). Then, for special mappings for the CMB
like Christoph is talking about, it's simply a matter of doing a range
compare on the PCI Bus address and converting the bus address to a BAR
and offset.
Logan
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Dan Williams @ 2019-06-26 20:39 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Logan Gunthorpe, Christoph Hellwig, Linux Kernel Mailing List,
linux-block, linux-nvme, linux-pci, linux-rdma, Jens Axboe,
Bjorn Helgaas, Sagi Grimberg, Keith Busch, Stephen Bates
In-Reply-To: <20190626202107.GA5850@ziepe.ca>
On Wed, Jun 26, 2019 at 1:21 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Wed, Jun 26, 2019 at 12:31:08PM -0600, Logan Gunthorpe wrote:
> > > we have a hole behind len where we could store flag. Preferably
> > > optionally based on a P2P or other magic memory types config
> > > option so that 32-bit systems with 32-bit phys_addr_t actually
> > > benefit from the smaller and better packing structure.
> >
> > That seems sensible. The one thing that's unclear though is how to get
> > the PCI Bus address when appropriate. Can we pass that in instead of the
> > phys_addr with an appropriate flag? Or will we need to pass the actual
> > physical address and then, at the map step, the driver has to some how
> > lookup the PCI device to figure out the bus offset?
>
> I agree with CH, if we go down this path it is a layering violation
> for the thing injecting bio's into the block stack to know what struct
> device they egress&dma map on just to be able to do the dma_map up
> front.
>
> So we must be able to go from this new phys_addr_t&flags to some BAR
> information during dma_map.
>
> For instance we could use a small hash table of the upper phys addr
> bits, or an interval tree, to do the lookup.
Hmm, that sounds like dev_pagemap without the pages.
There's already no requirement that dev_pagemap point to real /
present pages (DEVICE_PRIVATE) seems a straightforward extension to
use it for helping coordinate phys_addr_t in 'struct bio'. Then
Logan's future plans to let userspace coordinate p2p operations could
build on PTE_DEVMAP.
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Jason Gunthorpe @ 2019-06-26 20:21 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: Christoph Hellwig, linux-kernel, linux-block, linux-nvme,
linux-pci, linux-rdma, Jens Axboe, Bjorn Helgaas, Dan Williams,
Sagi Grimberg, Keith Busch, Stephen Bates
In-Reply-To: <c15d5997-9ba4-f7db-0e7a-a69e75df316c@deltatee.com>
On Wed, Jun 26, 2019 at 12:31:08PM -0600, Logan Gunthorpe wrote:
> > we have a hole behind len where we could store flag. Preferably
> > optionally based on a P2P or other magic memory types config
> > option so that 32-bit systems with 32-bit phys_addr_t actually
> > benefit from the smaller and better packing structure.
>
> That seems sensible. The one thing that's unclear though is how to get
> the PCI Bus address when appropriate. Can we pass that in instead of the
> phys_addr with an appropriate flag? Or will we need to pass the actual
> physical address and then, at the map step, the driver has to some how
> lookup the PCI device to figure out the bus offset?
I agree with CH, if we go down this path it is a layering violation
for the thing injecting bio's into the block stack to know what struct
device they egress&dma map on just to be able to do the dma_map up
front.
So we must be able to go from this new phys_addr_t&flags to some BAR
information during dma_map.
For instance we could use a small hash table of the upper phys addr
bits, or an interval tree, to do the lookup.
The bar info would give the exporting struct device and any other info
we need to make the iommu mapping.
This phys_addr_t seems like a good approach to me as it avoids the
struct page overheads and will lets us provide copy from/to bio
primitives that could work on BAR memory. I think we can surely use
this approach in RDMA as well.
Jason
^ permalink raw reply
* Re: [PATCH V2 mlx5-next 00/13] Mellanox, mlx5 vport metadata matching
From: Saeed Mahameed @ 2019-06-26 19:05 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190625174727.20309-1-saeedm@mellanox.com>
On Tue, 2019-06-25 at 17:47 +0000, Saeed Mahameed wrote:
> This series includes mlx5 updates for both rdma and net-next trees.
> In case of no objection it will be applied to mlx5-next branch and
> later
> on will be sent as pull request to rdma and net-next.
>
> From Jianbo, Vport meta data matching:
>
> Hardware steering has no notion of vport number, and vport is an
> abstract concept, so firmware need to translate the source vport
> matching to match on the VHCA ID (Virtual HCA ID).
>
> In dual-port RoCE, the dual-port VHCA is able to send also on the
> second port on behalf of the affiliated vport, so now we can’t assume
> anymore that vport is represented by single VHCA only.
>
> To resolve this issue, we use metadata register as source port
> indicator instead.
>
> When a packet enters the eswitch, eswitch ingress traffic passes the
> ingress ACL flow tables, where we tag the packets (via the metadata
> value, in this case REG_C at index 0) with a unique value which will
> act as an alias of the vport. In order to guarantee uniqueness, we
> use
> the eswitch owner vhca id and the vport number as that value.
>
> Usually, the vports are numbered in each eswitch as followed:
> - Physical Function (PF) vport, the number is 0.
> - Virtual Function (VF) vport, starting from 1.
> - Uplink vport, the reserved vport number for it is 0xFFFF.
>
> With the metadata in each packet, we can then do matching on it, in
> both fast path and slow path.
>
> For slow path, there is a representor for each vport. Packet that
> misses all offloaded rules in FDB, will be forwarded to the eswitch
> manager vport. In its NIC RX, it then will be steered to the right
> representor. The rules, which decide the destination representor,
> previously were matching on source port, will now match metadata
> instead.
>
Series applied to mlx5-next.
Thanks everyone!
Saeed.
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Logan Gunthorpe @ 2019-06-26 18:31 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel, linux-block, linux-nvme, linux-pci, linux-rdma,
Jens Axboe, Bjorn Helgaas, Dan Williams, Sagi Grimberg,
Keith Busch, Jason Gunthorpe, Stephen Bates
In-Reply-To: <20190626065708.GB24531@lst.de>
On 2019-06-26 12:57 a.m., Christoph Hellwig wrote:
> On Tue, Jun 25, 2019 at 01:54:21PM -0600, Logan Gunthorpe wrote:
>> Well whether it's dma_addr_t, phys_addr_t, pfn_t the result isn't all
>> that different. You still need roughly the same 'if' hooks for any
>> backed memory that isn't in the linear mapping and you can't get a
>> kernel mapping for directly.
>>
>> It wouldn't be too hard to do a similar patch set that uses something
>> like phys_addr_t instead and have a request and queue flag for support
>> of non-mappable memory. But you'll end up with very similar 'if' hooks
>> and we'd have to clean up all bio-using drivers that access the struct
>> pages directly.
>
> We'll need to clean that mess up anyway, and I've been chugging
> along doing some of that. A lot still assume no highmem, so we need
> to convert them over to something that kmaps anyway. If we get
> the abstraction right that will actually help converting over to
> a better reprsentation.
>
>> Though, we'd also still have the problem of how to recognize when the
>> address points to P2PDMA and needs to be translated to the bus offset.
>> The map-first inversion was what helped here because the driver
>> submitting the requests had all the information. Though it could be
>> another request flag and indicating non-mappable memory could be a flag
>> group like REQ_NOMERGE_FLAGS -- REQ_NOMAP_FLAGS.
>
> The assumes the request all has the same memory, which is a simplifing
> assuption. My idea was that if had our new bio_vec like this:
>
> struct bio_vec {
> phys_addr_t paddr; // 64-bit on 64-bit systems
> unsigned long len;
> };
>
> we have a hole behind len where we could store flag. Preferably
> optionally based on a P2P or other magic memory types config
> option so that 32-bit systems with 32-bit phys_addr_t actually
> benefit from the smaller and better packing structure.
That seems sensible. The one thing that's unclear though is how to get
the PCI Bus address when appropriate. Can we pass that in instead of the
phys_addr with an appropriate flag? Or will we need to pass the actual
physical address and then, at the map step, the driver has to some how
lookup the PCI device to figure out the bus offset?
>> If you think any of the above ideas sound workable I'd be happy to try
>> to code up another prototype.
>
> Іt sounds workable. To some of the first steps are cleanups independent
> of how the bio_vec is eventually going to look like. That is making
> the DMA-API internals work on the phys_addr_t, which also unifies the
> map_resource implementation with map_page. I plan to do that relatively
> soon. The next is sorting out access to bios data by virtual address.
> All these need nice kmapping helper that avoid too much open coding.
> I was going to look into that next, mostly to kill the block layer
> bounce buffering code. Similar things will also be needed at the
> scatterlist level I think. After that we need to more audits of
> how bv_page is still used. something like a bv_phys() helper that
> does "page_to_phys(bv->bv_page) + bv->bv_offset" might come in handy
> for example.
Ok, I should be able to help with that. When I have a chance I'll try to
look at the bv_phys() helper.
Logan
^ permalink raw reply
* Re: [PATCH v4 hmm 12/12] mm/hmm: Fix error flows in hmm_invalidate_range_start
From: Ralph Campbell @ 2019-06-26 18:18 UTC (permalink / raw)
To: Jason Gunthorpe, Jerome Glisse, John Hubbard, Felix.Kuehling
Cc: Andrea Arcangeli, Philip Yang, linux-rdma, amd-gfx, linux-mm,
Jason Gunthorpe, dri-devel, Ira Weiny, Christoph Hellwig,
Ben Skeggs
In-Reply-To: <20190624210110.5098-13-jgg@ziepe.ca>
On 6/24/19 2:01 PM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> If the trylock on the hmm->mirrors_sem fails the function will return
> without decrementing the notifiers that were previously incremented. Since
> the caller will not call invalidate_range_end() on EAGAIN this will result
> in notifiers becoming permanently incremented and deadlock.
>
> If the sync_cpu_device_pagetables() required blocking the function will
> not return EAGAIN even though the device continues to touch the
> pages. This is a violation of the mmu notifier contract.
>
> Switch, and rename, the ranges_lock to a spin lock so we can reliably
> obtain it without blocking during error unwind.
>
> The error unwind is necessary since the notifiers count must be held
> incremented across the call to sync_cpu_device_pagetables() as we cannot
> allow the range to become marked valid by a parallel
> invalidate_start/end() pair while doing sync_cpu_device_pagetables().
>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Philip Yang <Philip.Yang@amd.com>
> ---
> include/linux/hmm.h | 2 +-
> mm/hmm.c | 72 +++++++++++++++++++++++++++------------------
> 2 files changed, 45 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index bf013e96525771..0fa8ea34ccef6d 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -86,7 +86,7 @@
> struct hmm {
> struct mm_struct *mm;
> struct kref kref;
> - struct mutex lock;
> + spinlock_t ranges_lock;
> struct list_head ranges;
> struct list_head mirrors;
> struct mmu_notifier mmu_notifier;
> diff --git a/mm/hmm.c b/mm/hmm.c
> index b224ea635a7716..89549eac03d506 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -64,7 +64,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> init_rwsem(&hmm->mirrors_sem);
> hmm->mmu_notifier.ops = NULL;
> INIT_LIST_HEAD(&hmm->ranges);
> - mutex_init(&hmm->lock);
> + spin_lock_init(&hmm->ranges_lock);
> kref_init(&hmm->kref);
> hmm->notifiers = 0;
> hmm->mm = mm;
> @@ -144,6 +144,23 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> hmm_put(hmm);
> }
>
> +static void notifiers_decrement(struct hmm *hmm)
> +{
> + lockdep_assert_held(&hmm->ranges_lock);
> +
Why not acquire the lock here and release at the end instead
of asserting the lock is held?
It looks like everywhere notifiers_decrement() is called does
that.
> + hmm->notifiers--;
> + if (!hmm->notifiers) {
> + struct hmm_range *range;
> +
> + list_for_each_entry(range, &hmm->ranges, list) {
> + if (range->valid)
> + continue;
> + range->valid = true;
> + }
> + wake_up_all(&hmm->wq);
> + }
> +}
> +
> static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> const struct mmu_notifier_range *nrange)
> {
> @@ -151,6 +168,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> struct hmm_mirror *mirror;
> struct hmm_update update;
> struct hmm_range *range;
> + unsigned long flags;
> int ret = 0;
>
> if (!kref_get_unless_zero(&hmm->kref))
> @@ -161,12 +179,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> update.event = HMM_UPDATE_INVALIDATE;
> update.blockable = mmu_notifier_range_blockable(nrange);
>
> - if (mmu_notifier_range_blockable(nrange))
> - mutex_lock(&hmm->lock);
> - else if (!mutex_trylock(&hmm->lock)) {
> - ret = -EAGAIN;
> - goto out;
> - }
> + spin_lock_irqsave(&hmm->ranges_lock, flags);
> hmm->notifiers++;
> list_for_each_entry(range, &hmm->ranges, list) {
> if (update.end < range->start || update.start >= range->end)
> @@ -174,7 +187,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn,
>
> range->valid = false;
> }
> - mutex_unlock(&hmm->lock);
> + spin_unlock_irqrestore(&hmm->ranges_lock, flags);
>
> if (mmu_notifier_range_blockable(nrange))
> down_read(&hmm->mirrors_sem);
> @@ -182,16 +195,26 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> ret = -EAGAIN;
> goto out;
> }
> +
> list_for_each_entry(mirror, &hmm->mirrors, list) {
> - int ret;
> + int rc;
>
> - ret = mirror->ops->sync_cpu_device_pagetables(mirror, &update);
> - if (!update.blockable && ret == -EAGAIN)
> + rc = mirror->ops->sync_cpu_device_pagetables(mirror, &update);
> + if (rc) {
> + if (WARN_ON(update.blockable || rc != -EAGAIN))
> + continue;
> + ret = -EAGAIN;
> break;
> + }
> }
> up_read(&hmm->mirrors_sem);
>
> out:
> + if (ret) {
> + spin_lock_irqsave(&hmm->ranges_lock, flags);
> + notifiers_decrement(hmm);
> + spin_unlock_irqrestore(&hmm->ranges_lock, flags);
> + }
> hmm_put(hmm);
> return ret;
> }
> @@ -200,23 +223,14 @@ static void hmm_invalidate_range_end(struct mmu_notifier *mn,
> const struct mmu_notifier_range *nrange)
> {
> struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> + unsigned long flags;
>
> if (!kref_get_unless_zero(&hmm->kref))
> return;
>
> - mutex_lock(&hmm->lock);
> - hmm->notifiers--;
> - if (!hmm->notifiers) {
> - struct hmm_range *range;
> -
> - list_for_each_entry(range, &hmm->ranges, list) {
> - if (range->valid)
> - continue;
> - range->valid = true;
> - }
> - wake_up_all(&hmm->wq);
> - }
> - mutex_unlock(&hmm->lock);
> + spin_lock_irqsave(&hmm->ranges_lock, flags);
> + notifiers_decrement(hmm);
> + spin_unlock_irqrestore(&hmm->ranges_lock, flags);
>
> hmm_put(hmm);
> }
> @@ -868,6 +882,7 @@ int hmm_range_register(struct hmm_range *range,
> {
> unsigned long mask = ((1UL << page_shift) - 1UL);
> struct hmm *hmm = mirror->hmm;
> + unsigned long flags;
>
> range->valid = false;
> range->hmm = NULL;
> @@ -886,7 +901,7 @@ int hmm_range_register(struct hmm_range *range,
> return -EFAULT;
>
> /* Initialize range to track CPU page table updates. */
> - mutex_lock(&hmm->lock);
> + spin_lock_irqsave(&hmm->ranges_lock, flags);
>
> range->hmm = hmm;
> kref_get(&hmm->kref);
> @@ -898,7 +913,7 @@ int hmm_range_register(struct hmm_range *range,
> */
> if (!hmm->notifiers)
> range->valid = true;
> - mutex_unlock(&hmm->lock);
> + spin_unlock_irqrestore(&hmm->ranges_lock, flags);
>
> return 0;
> }
> @@ -914,10 +929,11 @@ EXPORT_SYMBOL(hmm_range_register);
> void hmm_range_unregister(struct hmm_range *range)
> {
> struct hmm *hmm = range->hmm;
> + unsigned long flags;
>
> - mutex_lock(&hmm->lock);
> + spin_lock_irqsave(&hmm->ranges_lock, flags);
> list_del_init(&range->list);
> - mutex_unlock(&hmm->lock);
> + spin_unlock_irqrestore(&hmm->ranges_lock, flags);
>
> /* Drop reference taken by hmm_range_register() */
> mmput(hmm->mm);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
From: Khalid Aziz @ 2019-06-26 17:50 UTC (permalink / raw)
To: Andrey Konovalov, linux-arm-kernel, linux-mm, linux-kernel,
amd-gfx, dri-devel, linux-rdma, linux-media, kvm, linux-kselftest
Cc: Mark Rutland, Szabolcs Nagy, Catalin Marinas, Will Deacon,
Kostya Serebryany, Felix Kuehling, Vincenzo Frascino,
Jacob Bramley, Leon Romanovsky, Christoph Hellwig,
Jason Gunthorpe, Dave Martin, Evgeniy Stepanov, Kevin Brodsky,
Kees Cook, Ruben Ayrapetyan, Ramana Radhakrishnan,
Alex Williamson, Mauro Carvalho Chehab, Dmitry Vyukov,
Greg Kroah-Hartman, Yishai Hadas
In-Reply-To: <61d800c35a4f391218fbca6f05ec458557d8d097.1561386715.git.andreyknvl@google.com>
On 6/24/19 8:32 AM, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged)
> userspace pointer. The untagged address should be used so that MMU
> notifiers for the untagged address get correctly matched up with the right
> BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses
> provided user pointers for vma lookups, which can only by done with
> untagged pointers.
>
> This patch untags user pointers in radeon_gem_userptr_ioctl().
>
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
> drivers/gpu/drm/radeon/radeon_gem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 44617dec8183..90eb78fb5eb2 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
> uint32_t handle;
> int r;
>
> + args->addr = untagged_addr(args->addr);
> +
> if (offset_in_page(args->addr | args->size))
> return -EINVAL;
>
>
^ permalink raw reply
* Re: [PATCH v18 00/15] arm64: untag user pointers passed to the kernel
From: Catalin Marinas @ 2019-06-26 17:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Konovalov, linux-arm-kernel, linux-mm, linux-kernel,
amd-gfx, dri-devel, linux-rdma, linux-media, kvm, linux-kselftest,
Vincenzo Frascino, Will Deacon, Mark Rutland, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
Hi Andrew,
On Mon, Jun 24, 2019 at 04:32:45PM +0200, Andrey Konovalov wrote:
> Andrey Konovalov (14):
> arm64: untag user pointers in access_ok and __uaccess_mask_ptr
> lib: untag user pointers in strn*_user
> mm: untag user pointers passed to memory syscalls
> mm: untag user pointers in mm/gup.c
> mm: untag user pointers in get_vaddr_frames
> fs/namespace: untag user pointers in copy_mount_options
> userfaultfd: untag user pointers
> drm/amdgpu: untag user pointers
> drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
> IB/mlx4: untag user pointers in mlx4_get_umem_mr
> media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get
> tee/shm: untag user pointers in tee_shm_register
> vfio/type1: untag user pointers in vaddr_get_pfn
> selftests, arm64: add a selftest for passing tagged pointers to kernel
>
> Catalin Marinas (1):
> arm64: Introduce prctl() options to control the tagged user addresses
> ABI
>
> arch/arm64/Kconfig | 9 +++
> arch/arm64/include/asm/processor.h | 8 +++
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/include/asm/uaccess.h | 12 +++-
> arch/arm64/kernel/process.c | 72 +++++++++++++++++++
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +
> drivers/gpu/drm/radeon/radeon_gem.c | 2 +
> drivers/infiniband/hw/mlx4/mr.c | 7 +-
> drivers/media/v4l2-core/videobuf-dma-contig.c | 9 +--
> drivers/tee/tee_shm.c | 1 +
> drivers/vfio/vfio_iommu_type1.c | 2 +
> fs/namespace.c | 2 +-
> fs/userfaultfd.c | 22 +++---
> include/uapi/linux/prctl.h | 5 ++
> kernel/sys.c | 12 ++++
> lib/strncpy_from_user.c | 3 +-
> lib/strnlen_user.c | 3 +-
> mm/frame_vector.c | 2 +
> mm/gup.c | 4 ++
> mm/madvise.c | 2 +
> mm/mempolicy.c | 3 +
> mm/migrate.c | 2 +-
> mm/mincore.c | 2 +
> mm/mlock.c | 4 ++
> mm/mprotect.c | 2 +
> mm/mremap.c | 7 ++
> mm/msync.c | 2 +
> tools/testing/selftests/arm64/.gitignore | 1 +
> tools/testing/selftests/arm64/Makefile | 11 +++
> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++
> tools/testing/selftests/arm64/tags_test.c | 29 ++++++++
> 32 files changed, 232 insertions(+), 25 deletions(-)
It looks like we got to an agreement on how to deal with tagged user
addresses between SPARC ADI and ARM Memory Tagging. If there are no
other objections, what's your preferred way of getting this series into
-next first and then mainline? Are you ok to merge them into the mm
tree?
Thanks.
--
Catalin
^ permalink raw reply
* [PATCH] net/mlx5: remove redundant assignment to ret
From: Colin King @ 2019-06-26 16:44 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky, David S . Miller, netdev,
linux-rdma
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Variable ret is being initialized with a value that is never read and
ret is being re-assigned later on. The initialization is redundant
and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/mellanox/mlx5/core/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 072b56fda27e..dd47c6d03dad 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1477,7 +1477,7 @@ static const struct pci_error_handlers mlx5_err_handler = {
static int mlx5_try_fast_unload(struct mlx5_core_dev *dev)
{
bool fast_teardown = false, force_teardown = false;
- int ret = 1;
+ int ret;
fast_teardown = MLX5_CAP_GEN(dev, fast_teardown);
force_teardown = MLX5_CAP_GEN(dev, force_teardown);
--
2.20.1
^ permalink raw reply related
* Re: [for-next V2 08/10] linux/dim: Implement rdma_dim
From: Or Gerlitz @ 2019-06-26 11:57 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Saeed Mahameed, David S. Miller, Doug Ledford, Jason Gunthorpe,
Leon Romanovsky, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Yamin Friedman, Max Gurtovoy,
Idan Burstein, Or Gerlitz
In-Reply-To: <bfa2159e-1576-6b3c-c85b-ee98bd4f9a47@grimberg.me>
On Wed, Jun 26, 2019 at 1:03 AM Sagi Grimberg <sagi@grimberg.me> wrote:
>
> > +void rdma_dim(struct dim *dim, u64 completions)
> > +{
> > + struct dim_sample *curr_sample = &dim->measuring_sample;
> > + struct dim_stats curr_stats;
> > + u32 nevents;
> > +
> > + dim_update_sample_with_comps(curr_sample->event_ctr + 1,
> > + curr_sample->pkt_ctr,
> > + curr_sample->byte_ctr,
> > + curr_sample->comp_ctr + completions,
> > + &dim->measuring_sample);
>
> If this is the only caller, why add pkt_ctr and byte_ctr at all?
Hi Sagi,
Thanks for the fast review and feedback, other than the default per
ib/rdma device setup for rdma
dim / adaptive-moderation for which Idan commented on (and lets
discuss it there please) seems
the rest of the comments are fine and Yamin will respond / address
them in the coming days.
Or.
^ permalink raw reply
* RE: [for-next V2 10/10] RDMA/core: Provide RDMA DIM support for ULPs
From: Idan Burstein @ 2019-06-26 7:56 UTC (permalink / raw)
To: Sagi Grimberg, Saeed Mahameed, David S. Miller, Doug Ledford,
Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Yamin Friedman, Max Gurtovoy
In-Reply-To: <adb3687a-6db3-b1a4-cd32-8b4889550c81@grimberg.me>
" Please don't. This is a bad choice to opt it in by default."
I disagree here. I'd prefer Linux to have good out of the box experience (e.g. reach 100G in 4K NVMeOF on Intel servers) with the default parameters. Especially since Yamin have shown it is beneficial / not hurting in terms of performance for variety of use cases. The whole concept of DIM is that it adapts to the workload requirements in terms of bandwidth and latency.
Moreover, net-dim is enabled by default, I don't see why RDMA is different.
-----Original Message-----
From: linux-rdma-owner@vger.kernel.org <linux-rdma-owner@vger.kernel.org> On Behalf Of Sagi Grimberg
Sent: Wednesday, June 26, 2019 12:14 AM
To: Saeed Mahameed <saeedm@mellanox.com>; David S. Miller <davem@davemloft.net>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@mellanox.com>
Cc: Leon Romanovsky <leonro@mellanox.com>; Or Gerlitz <ogerlitz@mellanox.com>; Tal Gilboa <talgi@mellanox.com>; netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Yamin Friedman <yaminf@mellanox.com>; Max Gurtovoy <maxg@mellanox.com>
Subject: Re: [for-next V2 10/10] RDMA/core: Provide RDMA DIM support for ULPs
> +static int ib_poll_dim_handler(struct irq_poll *iop, int budget) {
> + struct ib_cq *cq = container_of(iop, struct ib_cq, iop);
> + struct dim *dim = cq->dim;
> + int completed;
> +
> + completed = __ib_process_cq(cq, budget, cq->wc, IB_POLL_BATCH);
> + if (completed < budget) {
> + irq_poll_complete(&cq->iop);
> + if (ib_req_notify_cq(cq, IB_POLL_FLAGS) > 0)
> + irq_poll_sched(&cq->iop);
> + }
> +
> + rdma_dim(dim, completed);
Why duplicate the entire thing for a one-liner?
> +
> + return completed;
> +}
> +
> static void ib_cq_completion_softirq(struct ib_cq *cq, void *private)
> {
> irq_poll_sched(&cq->iop);
> @@ -105,14 +157,18 @@ static void ib_cq_completion_softirq(struct
> ib_cq *cq, void *private)
>
> static void ib_cq_poll_work(struct work_struct *work)
> {
> - struct ib_cq *cq = container_of(work, struct ib_cq, work);
> + struct ib_cq *cq = container_of(work, struct ib_cq,
> + work);
Why was that changed?
> int completed;
>
> completed = __ib_process_cq(cq, IB_POLL_BUDGET_WORKQUEUE, cq->wc,
> IB_POLL_BATCH);
> +
newline?
> if (completed >= IB_POLL_BUDGET_WORKQUEUE ||
> ib_req_notify_cq(cq, IB_POLL_FLAGS) > 0)
> queue_work(cq->comp_wq, &cq->work);
> + else if (cq->dim)
> + rdma_dim(cq->dim, completed);
> }
>
> static void ib_cq_completion_workqueue(struct ib_cq *cq, void
> *private) @@ -166,6 +222,8 @@ struct ib_cq *__ib_alloc_cq_user(struct ib_device *dev, void *private,
> rdma_restrack_set_task(&cq->res, caller);
> rdma_restrack_kadd(&cq->res);
>
> + rdma_dim_init(cq);
> +
> switch (cq->poll_ctx) {
> case IB_POLL_DIRECT:
> cq->comp_handler = ib_cq_completion_direct; @@ -173,7 +231,13 @@
> struct ib_cq *__ib_alloc_cq_user(struct ib_device *dev, void *private,
> case IB_POLL_SOFTIRQ:
> cq->comp_handler = ib_cq_completion_softirq;
>
> - irq_poll_init(&cq->iop, IB_POLL_BUDGET_IRQ, ib_poll_handler);
> + if (cq->dim) {
> + irq_poll_init(&cq->iop, IB_POLL_BUDGET_IRQ,
> + ib_poll_dim_handler);
> + } else
> + irq_poll_init(&cq->iop, IB_POLL_BUDGET_IRQ,
> + ib_poll_handler);
> +
> ib_req_notify_cq(cq, IB_CQ_NEXT_COMP);
> break;
> case IB_POLL_WORKQUEUE:
> @@ -226,6 +290,9 @@ void ib_free_cq_user(struct ib_cq *cq, struct ib_udata *udata)
> WARN_ON_ONCE(1);
> }
>
> + if (cq->dim)
> + cancel_work_sync(&cq->dim->work);
> + kfree(cq->dim);
> kfree(cq->wc);
> rdma_restrack_del(&cq->res);
> ret = cq->device->ops.destroy_cq(cq, udata); diff --git
> a/drivers/infiniband/hw/mlx5/main.c
> b/drivers/infiniband/hw/mlx5/main.c
> index abac70ad5c7c..b1b45dbe24a5 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -6305,6 +6305,8 @@ static int mlx5_ib_stage_caps_init(struct mlx5_ib_dev *dev)
> MLX5_CAP_GEN(dev->mdev, disable_local_lb_mc)))
> mutex_init(&dev->lb.mutex);
>
> + dev->ib_dev.use_cq_dim = true;
> +
Please don't. This is a bad choice to opt it in by default.
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Christoph Hellwig @ 2019-06-26 6:57 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: Christoph Hellwig, linux-kernel, linux-block, linux-nvme,
linux-pci, linux-rdma, Jens Axboe, Bjorn Helgaas, Dan Williams,
Sagi Grimberg, Keith Busch, Jason Gunthorpe, Stephen Bates
In-Reply-To: <41235a05-8ed1-e69a-e7cd-48cae7d8a676@deltatee.com>
On Tue, Jun 25, 2019 at 01:54:21PM -0600, Logan Gunthorpe wrote:
> Well whether it's dma_addr_t, phys_addr_t, pfn_t the result isn't all
> that different. You still need roughly the same 'if' hooks for any
> backed memory that isn't in the linear mapping and you can't get a
> kernel mapping for directly.
>
> It wouldn't be too hard to do a similar patch set that uses something
> like phys_addr_t instead and have a request and queue flag for support
> of non-mappable memory. But you'll end up with very similar 'if' hooks
> and we'd have to clean up all bio-using drivers that access the struct
> pages directly.
We'll need to clean that mess up anyway, and I've been chugging
along doing some of that. A lot still assume no highmem, so we need
to convert them over to something that kmaps anyway. If we get
the abstraction right that will actually help converting over to
a better reprsentation.
> Though, we'd also still have the problem of how to recognize when the
> address points to P2PDMA and needs to be translated to the bus offset.
> The map-first inversion was what helped here because the driver
> submitting the requests had all the information. Though it could be
> another request flag and indicating non-mappable memory could be a flag
> group like REQ_NOMERGE_FLAGS -- REQ_NOMAP_FLAGS.
The assumes the request all has the same memory, which is a simplifing
assuption. My idea was that if had our new bio_vec like this:
struct bio_vec {
phys_addr_t paddr; // 64-bit on 64-bit systems
unsigned long len;
};
we have a hole behind len where we could store flag. Preferably
optionally based on a P2P or other magic memory types config
option so that 32-bit systems with 32-bit phys_addr_t actually
benefit from the smaller and better packing structure.
> If you think any of the above ideas sound workable I'd be happy to try
> to code up another prototype.
Іt sounds workable. To some of the first steps are cleanups independent
of how the bio_vec is eventually going to look like. That is making
the DMA-API internals work on the phys_addr_t, which also unifies the
map_resource implementation with map_page. I plan to do that relatively
soon. The next is sorting out access to bios data by virtual address.
All these need nice kmapping helper that avoid too much open coding.
I was going to look into that next, mostly to kill the block layer
bounce buffering code. Similar things will also be needed at the
scatterlist level I think. After that we need to more audits of
how bv_page is still used. something like a bv_phys() helper that
does "page_to_phys(bv->bv_page) + bv->bv_offset" might come in handy
for example.
^ permalink raw reply
* Re: [for-next V2 05/10] linux/dim: Rename externally used net_dim members
From: Tal Gilboa @ 2019-06-26 6:38 UTC (permalink / raw)
To: Sagi Grimberg, Saeed Mahameed, David S. Miller, Doug Ledford,
Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <c97bbab4-13a9-b9e1-69f2-d4aba43e1c06@grimberg.me>
On 6/26/2019 12:57 AM, Sagi Grimberg wrote:
> Question, do any other nics use or plan to use this?
Yes, see the changed files list under drivers/net for existing usage.
>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply
* Re: [for-next V2 08/10] linux/dim: Implement rdma_dim
From: Sagi Grimberg @ 2019-06-25 22:02 UTC (permalink / raw)
To: Saeed Mahameed, David S. Miller, Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Yamin Friedman, Max Gurtovoy
In-Reply-To: <20190625205701.17849-9-saeedm@mellanox.com>
> +void rdma_dim(struct dim *dim, u64 completions)
> +{
> + struct dim_sample *curr_sample = &dim->measuring_sample;
> + struct dim_stats curr_stats;
> + u32 nevents;
> +
> + dim_update_sample_with_comps(curr_sample->event_ctr + 1,
> + curr_sample->pkt_ctr,
> + curr_sample->byte_ctr,
> + curr_sample->comp_ctr + completions,
> + &dim->measuring_sample);
If this is the only caller, why add pkt_ctr and byte_ctr at all?
^ permalink raw reply
* Re: [for-next V2 05/10] linux/dim: Rename externally used net_dim members
From: Sagi Grimberg @ 2019-06-25 21:57 UTC (permalink / raw)
To: Saeed Mahameed, David S. Miller, Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <20190625205701.17849-6-saeedm@mellanox.com>
Question, do any other nics use or plan to use this?
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply
* Re: [for-next V2 04/10] linux/dim: Rename net_dim_sample() to net_dim_update_sample()
From: Sagi Grimberg @ 2019-06-25 21:55 UTC (permalink / raw)
To: Saeed Mahameed, David S. Miller, Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <20190625205701.17849-5-saeedm@mellanox.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply
* Re: [for-next V2 03/10] linux/dim: Rename externally exposed macros
From: Sagi Grimberg @ 2019-06-25 21:54 UTC (permalink / raw)
To: Saeed Mahameed, David S. Miller, Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <20190625205701.17849-4-saeedm@mellanox.com>
This can be squashed to the prev one, otherwise
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox