[ewg] RE: [ofa-general] [PATCH 5/5] nes: napi interface fix

Sean Hefty sean.hefty at intel.com
Fri Nov 30 10:50:23 PST 2007


>--- a/drivers/infiniband/hw/nes/nes_hw.c
>+++ b/drivers/infiniband/hw/nes/nes_hw.c
>@@ -1232,6 +1232,12 @@ static void nes_replenish_nic_rq(struct nes_vnic
>*nesvnic)
> 	nesnic = &nesvnic->nic;
> 	nesdev = nesvnic->nesdev;
> 	spin_lock_irqsave(&nesnic->rq_lock, flags);
>+	if (nesnic->replenishing_rq !=0) {
>+		spin_unlock_irqrestore(&nesnic->rq_lock, flags);
>+		return;
>+	}
>+	nesnic->replenishing_rq = 1;
>+	spin_unlock_irqrestore(&nesnic->rq_lock, flags);
> 	do {
> 		skb = dev_alloc_skb(nesvnic->max_frame_size);
> 		if (skb) {
>@@ -1275,7 +1281,7 @@ static void nes_replenish_nic_rq(struct nes_vnic
>*nesvnic)
> 	if (rx_wqes_posted) {
> 		nes_write32(nesdev->regs+NES_WQE_ALLOC, (rx_wqes_posted << 24) |
>nesnic->qp_id);
> 	}
>-	spin_unlock_irqrestore(&nesnic->rq_lock, flags);
>+	nesnic->replenishing_rq = 0;

It seems racy that this is set under lock, but cleared without the lock held.
How do you ensure that the nic_rq will always be replenished?

>@@ -2314,11 +2313,11 @@ void nes_nic_ce_handler(struct nes_device *nesdev,
>struct nes_hw_nic_cq *cq)
> 				cqe_count = 0;
> 			}
> #ifdef NES_NAPI

Is #ifdef napi sprinkled throughout the code common for most drivers?  Is there
a better way to handle this?  (Is this OFED only for backports, or for
upstream?)

>@@ -361,6 +361,7 @@ enum nes_cqe_opcode_bits {
> 	NES_CQE_VALID = (1<<31),
> };
>
>+
> enum nes_cqe_word_idx {
> 	NES_CQE_PAYLOAD_LENGTH_IDX = 0,
> 	NES_CQE_COMP_COMP_CTX_LOW_IDX = 2,
>@@ -810,6 +811,7 @@ struct nes_hw_aeqe {
> 	__le32 aeqe_words[4];
> };
>
>+

couple extra blank lines were added

> struct nes_cqp_request {
> 	wait_queue_head_t     waitq;
> 	struct nes_hw_cqp_wqe cqp_wqe;
>@@ -857,6 +859,8 @@ struct nes_hw_nic {
> 	u16 rq_head;
> 	u16 rq_tail;
> 	u16 rq_size;
>+	u8 replenishing_rq;
>+	u8 reserved;

Why is reserved added?

- Sean



More information about the ewg mailing list