RE: [patch 34/44] rxe_arbiter.c

["Bob Pearson" <rpearson-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org>]

A regression slipped in. We had been clean on sparse and checkpatch except
for the msleep which is not ciritical to performance.
CHECK_ENDIAN is new to me. That will help.

-----Original Message-----
From: bart.vanassche-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org [mailto:bart.vanassche-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org] On Behalf
Of Bart Van Assche
Sent: Sunday, July 03, 2011 2:53 AM
To: rpearson-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [patch 34/44] rxe_arbiter.c

On Fri, Jul 1, 2011 at 3:18 PM, <rpearson-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org> wrote:
>
> +int rxe_arbiter(void *arg)
> +{
> +       int err;
> +       unsigned long flags;
> +       struct rxe_dev *rxe = (struct rxe_dev *)arg;
> +       struct sk_buff *skb;
> +       struct list_head *qpl;
> +       struct rxe_qp *qp;
> +
> +       /* get the next qp's send queue */
> +       spin_lock_irqsave(&rxe->arbiter.list_lock, flags);
> +       if (list_empty(&rxe->arbiter.qp_list)) {
> +               spin_unlock_irqrestore(&rxe->arbiter.list_lock, 
> + flags);
> +               return 1;
> +       }
> +
> +       qpl = rxe->arbiter.qp_list.next;
> +       list_del_init(qpl);
> +       qp = list_entry(qpl, struct rxe_qp, arbiter_list);
> +       spin_unlock_irqrestore(&rxe->arbiter.list_lock, flags);
> +
> +       /* get next packet from queue and try to send it
> +          note skb could have already been removed */
> +       skb = skb_dequeue(&qp->send_pkts);
> +       if (skb) {
> +               err = xmit_one_packet(rxe, qp, skb);
> +               if (err) {
> +                       if (err == RXE_QUEUE_STOPPED)
> +                               skb_queue_head(&qp->send_pkts, skb);
> +                       rxe_run_task(&rxe->arbiter.task, 1);
> +                       
> + spin_unlock_irqrestore(&rxe->arbiter.list_lock, flags);
> +                       return 1;
> +               }
> +       }

Can you please run these patches through sparse ? Sparse complains about the
above code:

$ make C=2 M=drivers/infiniband/hw/rxe
[ ... ]
drivers/infiniband/hw/rxe/rxe_arbiter.c:155:25: warning: context imbalance
in 'rxe_arbiter' - unexpected unlock [ ... ]

Also, several complaints are reported with endianness checking enabled (make
CF=-D__CHECK_ENDIAN__ C=2 M=drivers/infiniband/hw/rxe).

Checkpatch is complaining too:

$ git show HEAD | scripts/checkpatch.pl - -nosignoff
WARNING: suspect code indent for conditional statements (8, 10)
#1061: FILE: drivers/infiniband/hw/rxe/rxe_arbiter.c:69:
+       if (is_request) {
+         if (qp->req.state != QP_STATE_READY)

WARNING: suspect code indent for conditional statements (10, 12)
#1062: FILE: drivers/infiniband/hw/rxe/rxe_arbiter.c:70:
+         if (qp->req.state != QP_STATE_READY)
+           goto drop;

WARNING: suspect code indent for conditional statements (10, 12)
#1065: FILE: drivers/infiniband/hw/rxe/rxe_arbiter.c:73:
+         if (qp->resp.state != QP_STATE_READY)
+           goto drop;

WARNING: msleep < 20ms can sleep for up to 20ms; see
Documentation/timers/timers-howto.txt
+               msleep(1);
total: 0 errors, 4 warnings, 14803 lines checked

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html