> -----Original Message-----
> From: Doug Ledford [mailto:dledf...@redhat.com]
> Sent: Tuesday, June 09, 2015 9:16 AM
> To: Steve Wise
> Cc: 'Hariprasad Shenai'; linux-rdma@vger.kernel.org; lee...@chelsio.com; 
> nirran...@chelsio.com
> Subject: Re: [PATCH for-4.1 2/2] iw_cxgb4: support for bar2 qid densities 
> exceeding the page size
> 
> On Tue, 2015-06-09 at 09:07 -0500, Steve Wise wrote:
> >
> > > -----Original Message-----
> > > From: Doug Ledford [mailto:dledf...@redhat.com]
> > > Sent: Tuesday, June 09, 2015 9:03 AM
> > > To: Hariprasad Shenai
> > > Cc: linux-rdma@vger.kernel.org; sw...@opengridcomputing.com; 
> > > lee...@chelsio.com; nirran...@chelsio.com
> > > Subject: Re: [PATCH for-4.1 2/2] iw_cxgb4: support for bar2 qid densities 
> > > exceeding the page size
> > >
> > > On Tue, 2015-06-09 at 18:23 +0530, Hariprasad Shenai wrote:
> > > > Handle this configuration:
> > > >
> > > >         Queues Per Page * SGE BAR2 Queue Register Area Size > Page Size
> > > >
> > > > Use cxgb4_bar2_sge_qregs() to obtain the proper location within the
> > > > bar2 region for a given qid.
> > > >
> > > > Rework the DB and GTS write functions to make use of this bar2 info.
> > > >
> > > > Signed-off-by: Steve Wise <sw...@opengridcomputing.com>
> > > > Signed-off-by: Hariprasad Shenai <haripra...@chelsio.com>
> > > > ---
> > > >  drivers/infiniband/hw/cxgb4/cq.c       | 22 ++++++------
> > > >  drivers/infiniband/hw/cxgb4/device.c   | 16 +++------
> > > >  drivers/infiniband/hw/cxgb4/iw_cxgb4.h |  5 +--
> > > >  drivers/infiniband/hw/cxgb4/qp.c       | 64 
> > > > ++++++++++++++++++++++------------
> > > >  drivers/infiniband/hw/cxgb4/t4.h       | 60 
> > > > ++++++++++++++++++++-----------
> > > >  5 files changed, 98 insertions(+), 69 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/hw/cxgb4/cq.c 
> > > > b/drivers/infiniband/hw/cxgb4/cq.c
> > > > index 68ddb37..8e5bbcb 100644
> > > > --- a/drivers/infiniband/hw/cxgb4/cq.c
> > > > +++ b/drivers/infiniband/hw/cxgb4/cq.c
> > > > @@ -156,19 +156,17 @@ static int create_cq(struct c4iw_rdev *rdev, 
> > > > struct t4_cq *cq,
> > > >                 goto err4;
> > > >
> > > >         cq->gen = 1;
> > > > +       cq->gts = rdev->lldi.gts_reg;
> > > >         cq->rdev = rdev;
> > > > -       if (user) {
> > > > -               u32 off = (cq->cqid << rdev->cqshift) & PAGE_MASK;
> > > >
> > > > -               cq->ugts = (u64)rdev->bar2_pa + off;
> > > > -       } else if (is_t4(rdev->lldi.adapter_type)) {
> > > > -               cq->gts = rdev->lldi.gts_reg;
> > > > -               cq->qid_mask = -1U;
> > > > -       } else {
> > > > -               u32 off = ((cq->cqid << rdev->cqshift) & PAGE_MASK) + 
> > > > 12;
> > > > -
> > > > -               cq->gts = rdev->bar2_kva + off;
> > > > -               cq->qid_mask = rdev->qpmask;
> > > > +       cq->bar2_va = c4iw_bar2_addrs(rdev, cq->cqid, 
> > > > T4_BAR2_QTYPE_INGRESS,
> > > > +                                     &cq->bar2_qid,
> > > > +                                     user ? &cq->bar2_pa : NULL);
> > > > +       if (user && !cq->bar2_va) {
> > > > +               pr_warn(MOD "%s: cqid %u not in BAR2 range.\n",
> > > > +                       pci_name(rdev->lldi.pdev), cq->cqid);
> > > > +               ret = -EINVAL;
> > > > +               goto err4;
> > > >         }
> > > >         return 0;
> > > >  err4:
> > > > @@ -971,7 +969,7 @@ struct ib_cq *c4iw_create_cq(struct ib_device 
> > > > *ibdev, int entries,
> > > >                 insert_mmap(ucontext, mm);
> > > >
> > > >                 mm2->key = uresp.gts_key;
> > > > -               mm2->addr = chp->cq.ugts;
> > > > +               mm2->addr = (u64)(uintptr_t)chp->cq.bar2_pa;
> > >
> > > Why are you using a cast here at all?  bar2_pa is already u64...
> > >
> >
> > So it should just have the (uintptr_t) cast?
> 
> No, it should be no cast at all.  The uintptr_t cast is only for casting
> an int->ptr or ptr->int.  In those cases, if the size of an int != size
> of ptr, you can loose data, and uintptr_t tells the compiler "I know I'm
> casting between possibly lossy data sizes and either A) I've checked and
> it's OK or B) I'm ok with ptr truncation and the loss won't hurt us".
> It basically turns off size checks when sticking a ptr into an int.  You
> should therefore use it only in those circumstances.  For example, when
> storing a cookie that doesn't have a strict uniqueness requirement, the
> loss due to truncation is probably OK.  Or if you know you are only
> doing something like initially storing an int into a pointer, and then
> later storing that pointer back into an int, so there can never be any
> truncation because the source of the ptr was always int sized.  Those
> are the times to use uintptr.  In this case, you have a real u64 going
> into a real u64, there should be no casts.
>

My bad.  I thought bar2_pa was a ptr...



--
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

Reply via email to