Skip to content

Commit

Permalink
bna: Add synchronization for tx ring.
Browse files Browse the repository at this point in the history
commit d667f78 upstream.

We received two reports of BUG_ON in bnad_txcmpl_process() where
hw_consumer_index appeared to be ahead of producer_index. Out of order
write/read of these variables could explain these reports.

bnad_start_xmit(), as a producer of tx descriptors, has a few memory
barriers sprinkled around writes to producer_index and the device's
doorbell but they're not paired with anything in bnad_txcmpl_process(), a
consumer.

Since we are synchronizing with a device, we must use mandatory barriers,
not smp_*. Also, I didn't see the purpose of the last smp_mb() in
bnad_start_xmit().

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
  • Loading branch information
Benjamin Poirier authored and Jiri Slaby committed Jan 27, 2017
1 parent e4f13c5 commit 32dacf4
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions drivers/net/ethernet/brocade/bna/bnad.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ bnad_txcmpl_process(struct bnad *bnad, struct bna_tcb *tcb)
return 0;

hw_cons = *(tcb->hw_consumer_index);
rmb();
cons = tcb->consumer_index;
q_depth = tcb->q_depth;

Expand Down Expand Up @@ -2906,13 +2907,12 @@ bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
BNA_QE_INDX_INC(prod, q_depth);
tcb->producer_index = prod;

smp_mb();
wmb();

if (unlikely(!test_bit(BNAD_TXQ_TX_STARTED, &tcb->flags)))
return NETDEV_TX_OK;

bna_txq_prod_indx_doorbell(tcb);
smp_mb();

return NETDEV_TX_OK;
}
Expand Down

0 comments on commit 32dacf4

Please sign in to comment.