On Wed, Nov 28, 2018 at 10:35:33AM -0800, Dennis Dalessandro wrote:
> From: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
>
> Sge sizing is done in several places using an open coded method.
>
> This can cause maintenance issues. The open coded method is
> encapsulated in a helper routine. The helper was introduced with
> commit:
>
> 1198fcea8a78 ("IB/hfi1, rdmavt: Move SGE state helper routines into
> rdmavt")
>
> Update all call sites that have the open coded path with the helper
> routine.
>
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxx>
> ---
> drivers/infiniband/hw/hfi1/verbs.c | 12 ++----------
> drivers/infiniband/hw/qib/qib_ud.c | 6 +-----
> drivers/infiniband/hw/qib/qib_verbs.c | 18 +++---------------
> 3 files changed, 6 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
> index 630f38e..55ec4db 100644
> --- a/drivers/infiniband/hw/hfi1/verbs.c
> +++ b/drivers/infiniband/hw/hfi1/verbs.c
> @@ -553,11 +553,7 @@ static noinline int build_verbs_ulp_payload(
> int ret = 0;
>
> while (length) {
> - len = ss->sge.length;
> - if (len > length)
> - len = length;
> - if (len > ss->sge.sge_length)
> - len = ss->sge.sge_length;
> + len = rvt_get_sge_length(&ss->sge, length);
> WARN_ON_ONCE(len == 0);
> ret = sdma_txadd_kvaddr(
> sde->dd,
> @@ -914,12 +910,8 @@ int hfi1_verbs_send_pio(struct rvt_qp *qp, struct hfi1_pkt_state *ps,
> if (ss) {
> while (len) {
> void *addr = ss->sge.vaddr;
> - u32 slen = ss->sge.length;
> + u32 slen = rvt_get_sge_length(&ss->sge, len);
>
> - if (slen > len)
> - slen = len;
> - if (slen > ss->sge.sge_length)
> - slen = ss->sge.sge_length;
> rvt_update_sge(ss, slen, false);
> seg_pio_copy_mid(pbuf, addr, slen);
> len -= slen;
> diff --git a/drivers/infiniband/hw/qib/qib_ud.c b/drivers/infiniband/hw/qib/qib_ud.c
> index 4d4c31e..8dab667 100644
> --- a/drivers/infiniband/hw/qib/qib_ud.c
> +++ b/drivers/infiniband/hw/qib/qib_ud.c
> @@ -172,12 +172,8 @@ static void qib_ud_loopback(struct rvt_qp *sqp, struct rvt_swqe *swqe)
> ssge.num_sge = swqe->wr.num_sge;
> sge = &ssge.sge;
> while (length) {
> - u32 len = sge->length;
> + u32 len = rvt_get_sge_length(sge, length);
>
> - if (len > length)
> - len = length;
> - if (len > sge->sge_length)
> - len = sge->sge_length;
> BUG_ON(len == 0);
^^^^^^, above you are using WARN_ON_ONCE and below BUG_ON
I assume that everything can be safely removed.
> rvt_copy_sge(qp, &qp->r_sge, sge->vaddr, len, true, false);
> sge->vaddr += len;
> diff --git a/drivers/infiniband/hw/qib/qib_verbs.c b/drivers/infiniband/hw/qib/qib_verbs.c
> index 4b0f576..9cbc644 100644
> --- a/drivers/infiniband/hw/qib/qib_verbs.c
> +++ b/drivers/infiniband/hw/qib/qib_verbs.c
> @@ -144,12 +144,8 @@ static u32 qib_count_sge(struct rvt_sge_state *ss, u32 length)
> u32 ndesc = 1; /* count the header */
>
> while (length) {
> - u32 len = sge.length;
> + u32 len = rvt_get_sge_length(&sge, length);
>
> - if (len > length)
> - len = length;
> - if (len > sge.sge_length)
> - len = sge.sge_length;
> BUG_ON(len == 0);
> if (((long) sge.vaddr & (sizeof(u32) - 1)) ||
> (len != length && (len & (sizeof(u32) - 1)))) {
> @@ -187,12 +183,8 @@ static void qib_copy_from_sge(void *data, struct rvt_sge_state *ss, u32 length)
> struct rvt_sge *sge = &ss->sge;
>
> while (length) {
> - u32 len = sge->length;
> + u32 len = rvt_get_sge_length(sge, length);
>
> - if (len > length)
> - len = length;
> - if (len > sge->sge_length)
> - len = sge->sge_length;
> BUG_ON(len == 0);
> memcpy(data, sge->vaddr, len);
> sge->vaddr += len;
> @@ -442,13 +434,9 @@ static void copy_io(u32 __iomem *piobuf, struct rvt_sge_state *ss,
> u32 last;
>
> while (1) {
> - u32 len = ss->sge.length;
> + u32 len = rvt_get_sge_length(&ss->sge, length);
> u32 off;
>
> - if (len > length)
> - len = length;
> - if (len > ss->sge.sge_length)
> - len = ss->sge.sge_length;
> BUG_ON(len == 0);
> /* If the source address is not aligned, try to align it. */
> off = (unsigned long)ss->sge.vaddr & (sizeof(u32) - 1);
>
Attachment:
signature.asc
Description: PGP signature
