* [PATCH] RDMA/cxgb3: test before subtraction on unsigned
@ 2009-03-03 13:38 Roel Kluin
2009-03-03 15:41 ` [ofa-general] " Steve Wise
0 siblings, 1 reply; 2+ messages in thread
From: Roel Kluin @ 2009-03-03 13:38 UTC (permalink / raw)
To: swise; +Cc: general, netdev, Andrew Morton
I think there's someting wrong in iwch_post_send():
// vi drivers/infiniband/hw/cxgb3/iwch_qp.c +353
int iwch_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
struct ib_send_wr **bad_wr)
{
...
u32 num_wrs;
...
num_wrs = Q_FREECNT(qhp->wq.sq_rptr, qhp->wq.sq_wptr,
qhp->wq.sq_size_log2);
if (num_wrs <= 0) {
spin_unlock_irqrestore(&qhp->lock, flag);
return -ENOMEM;
}
// vi drivers/infiniband/hw/cxgb3/cxio_wr.h +50
#define Q_FREECNT(rptr,wptr,size_log2) ((1UL<<size_log2)-((wptr)-(rptr)))
Since num_wrs is unsigned, I think the test should occur before the subtraction,
right? please review.
------------------------------>8-------------8<---------------------------------
num_wrs is unsigned so test before the subtraction.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/infiniband/hw/cxgb3/iwch_qp.c b/drivers/infiniband/hw/cxgb3/iwch_qp.c
index 19661b2..1c6ebaf 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_qp.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_qp.c
@@ -369,12 +369,13 @@ int iwch_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
spin_unlock_irqrestore(&qhp->lock, flag);
return -EINVAL;
}
- num_wrs = Q_FREECNT(qhp->wq.sq_rptr, qhp->wq.sq_wptr,
- qhp->wq.sq_size_log2);
- if (num_wrs <= 0) {
+ if ((1UL << qhp->wq.sq_size_log2) + qhp->wq.sq_rptr <=
+ qhp->wq.sq_wptr) {
spin_unlock_irqrestore(&qhp->lock, flag);
return -ENOMEM;
}
+ num_wrs = Q_FREECNT(qhp->wq.sq_rptr, qhp->wq.sq_wptr,
+ qhp->wq.sq_size_log2);
while (wr) {
if (num_wrs == 0) {
err = -ENOMEM;
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [ofa-general] Re: [PATCH] RDMA/cxgb3: test before subtraction on unsigned
2009-03-03 13:38 [PATCH] RDMA/cxgb3: test before subtraction on unsigned Roel Kluin
@ 2009-03-03 15:41 ` Steve Wise
0 siblings, 0 replies; 2+ messages in thread
From: Steve Wise @ 2009-03-03 15:41 UTC (permalink / raw)
To: Roel Kluin; +Cc: netdev, Andrew Morton, general, swise
Roel Kluin wrote:
> I think there's someting wrong in iwch_post_send():
>
> // vi drivers/infiniband/hw/cxgb3/iwch_qp.c +353
> int iwch_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
> struct ib_send_wr **bad_wr)
> {
> ...
> u32 num_wrs;
> ...
> num_wrs = Q_FREECNT(qhp->wq.sq_rptr, qhp->wq.sq_wptr,
> qhp->wq.sq_size_log2);
> if (num_wrs <= 0) {
> spin_unlock_irqrestore(&qhp->lock, flag);
> return -ENOMEM;
> }
>
> // vi drivers/infiniband/hw/cxgb3/cxio_wr.h +50
> #define Q_FREECNT(rptr,wptr,size_log2) ((1UL<<size_log2)-((wptr)-(rptr)))
>
> Since num_wrs is unsigned, I think the test should occur before the subtraction,
> right? please review.
> ------------------------------>8-------------8<---------------------------------
> num_wrs is unsigned so test before the subtraction.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/drivers/infiniband/hw/cxgb3/iwch_qp.c b/drivers/infiniband/hw/cxgb3/iwch_qp.c
> index 19661b2..1c6ebaf 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch_qp.c
> +++ b/drivers/infiniband/hw/cxgb3/iwch_qp.c
> @@ -369,12 +369,13 @@ int iwch_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
> spin_unlock_irqrestore(&qhp->lock, flag);
> return -EINVAL;
> }
> - num_wrs = Q_FREECNT(qhp->wq.sq_rptr, qhp->wq.sq_wptr,
> - qhp->wq.sq_size_log2);
> - if (num_wrs <= 0) {
> + if ((1UL << qhp->wq.sq_size_log2) + qhp->wq.sq_rptr <=
> + qhp->wq.sq_wptr) {
> spin_unlock_irqrestore(&qhp->lock, flag);
> return -ENOMEM;
> }
> + num_wrs = Q_FREECNT(qhp->wq.sq_rptr, qhp->wq.sq_wptr,
> + qhp->wq.sq_size_log2);
> while (wr) {
> if (num_wrs == 0) {
> err = -ENOMEM;
>
>
I think the test is just wrong. It should be:
if (num_wrs == 0).
Q_FREECNT() will not return < 0. The free count is either zero if it is
full, or non zero count of the number of free elements.
Steve.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2009-03-03 15:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-03 13:38 [PATCH] RDMA/cxgb3: test before subtraction on unsigned Roel Kluin
2009-03-03 15:41 ` [ofa-general] " Steve Wise
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).