Hello,

Your version of the patch won't work because for the SRQ case this will
always be 'TRUE':
> -             if (!qp->sq.wrid || !qp->rq.wrid) {
> -                     err = -ENOMEM;
> -                     goto err_wrid;

Thanks,
Alex.

> -----Original Message-----
> From: Leonid Keller [mailto:[EMAIL PROTECTED] 
> Sent: Sunday, September 28, 2008 11:13 AM
> To: Alex Estrin
> Cc: [email protected]
> Subject: RE: [ofw][mlx4] SRQ attached QP issue
> 
> Applied with some changes in 1625, thank you. 
> 
> > -----Original Message-----
> > From: Alex Estrin [mailto:[EMAIL PROTECTED] 
> > Sent: Saturday, September 27, 2008 1:20 AM
> > To: Leonid Keller
> > Cc: [email protected]
> > Subject: [ofw][mlx4] SRQ attached QP issue
> > 
> > Hello,
> > 
> > This patch seem fixed the create-QP-attached-to-SRQ issue.
> > I didn't verify though, if code allowes send queue zero size. 
> > Included check anyway.
> > Please review.
> > 
> > Thanks,
> > Alex.
> > 
> > Index: mlx4/kernel/bus/ib/qp.c
> > ===================================================================
> > --- mlx4/kernel/bus/ib/qp.c (revision 1622)
> > +++ mlx4/kernel/bus/ib/qp.c (working copy)
> > @@ -411,14 +411,20 @@
> >             err = mlx4_buf_write_mtt(dev->dev, &qp->mtt, &qp->buf);
> >             if (err)
> >                     goto err_mtt;
> > -
> > -           qp->sq.wrid  = kmalloc(qp->sq.wqe_cnt * sizeof (u64),
> > GFP_KERNEL);
> > -           qp->rq.wrid  = kmalloc(qp->rq.wqe_cnt * sizeof (u64),
> > GFP_KERNEL);
> > -
> > -           if (!qp->sq.wrid || !qp->rq.wrid) {
> > -                   err = -ENOMEM;
> > -                   goto err_wrid;
> > +           if (qp->sq.wqe_cnt) {
> > +                   qp->sq.wrid  = kmalloc(qp->sq.wqe_cnt * sizeof
> > (u64), GFP_KERNEL);
> > +                   if (!qp->sq.wrid) {
> > +                           err = -ENOMEM;
> > +                           goto err_wrid;
> > +                   }
> >             }
> > +           if (qp->rq.wqe_cnt) {
> > +                   qp->rq.wrid  = kmalloc(qp->rq.wqe_cnt * sizeof
> > (u64), GFP_KERNEL);
> > +                   if (!qp->rq.wrid) {
> > +                           err = -ENOMEM;
> > +                           goto err_wrid;
> > +                   }
> > +           }
> >     }
> >  
> >     if (!sqpn)
> > @@ -452,8 +458,10 @@
> >                     mlx4_ib_db_unmap_user(to_mucontext(pd->p_uctx),
> >                                           &qp->db);
> >     } else {
> > -           kfree(qp->sq.wrid);
> > -           kfree(qp->rq.wrid);
> > +           if (qp->sq.wrid)
> > +                   kfree(qp->sq.wrid);
> > +           if (qp->rq.wrid)
> > +                   kfree(qp->rq.wrid);
> >     }
> >  
> >  err_mtt:
> > 
> > 
> > 
> > > Hello,
> > > 
> > > It seem there is a controversy exist in mlx4 driver that 
> > won't allow 
> > > to create QP if it was attached to SRQ.
> > > 
> > > At first it doesn't allow to set any recv queue size 
> (which is fine)
> > > 
> > > mlx4\kernel\bus\ib\qp.c@ line 219:
> > >           if (cap->max_recv_wr)
> > >                   return -EINVAL;
> > > 
> > >           qp->rq.wqe_cnt = qp->rq.max_gs = 0;
> > > 
> > > But then there is assertion for zero size memory request in 
> > 'kmalloc':
> > > mlx4\kernel\bus\ib\qp.c @ line 416:
> > > 
> > >           qp->rq.wrid  = kmalloc(qp->rq.wqe_cnt * sizeof 
> > (u64), GFP_KERNEL);
> > >           
> > > mlx4\kernel\inc\l2w_memory.h @ line 81:
> > > 
> > >           static inline void * kmalloc( SIZE_T bsize, 
> > gfp_t gfp_mask)
> > >           {
> > >                   void *ptr;
> > >                   ASSERT( KeGetCurrentIrql() <= DISPATCH_LEVEL);
> > >                   ASSERT(bsize);
> > >           .....
> > > 
> > > I believe it was added for a reason.
> > > Please let me know if I missed anything.
> > > 
> > > Thanks,
> > > Alex.
> > > 
> > > 
> > > 
> > 
> 
_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw

Reply via email to