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.


-- 
Doug Ledford <dledf...@redhat.com>
              GPG KeyID: 0E572FDD

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to