Hello.
On 04/24/2015 09:53 PM, Sergei Shtylyov wrote:
[...]
+static int ravb_start_xmit(struct sk_buff *skb, struct net_device
*ndev)
+{
+ struct ravb_private *priv = netdev_priv(ndev);
+ struct ravb_tstamp_skb *ts_skb = NULL;
+ struct ravb_tx_desc *desc;
+ unsigned long flags;
+ void *buffer;
+ u32 entry;
+ u32 tccr;
+ int q;
+
+ /* If skb needs TX timestamp, it is handled in network control
queue */
+ q = (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) ? RAVB_NC :
RAVB_BE;
+
+ spin_lock_irqsave(&priv->lock, flags);
+ if (priv->cur_tx[q] - priv->dirty_tx[q] >= priv->num_tx_ring[q]
- 4) {
What's so special about 4 here, you don't seem to be using 4 descriptors
Not sure, this was clearly copied from sh_eth.c. Perhaps it's just a
threshold for calling ravb_tx_free()...
Then 1 inclusive or 0 exclusive is probably what you should be comparing
to, otherwise you may just stop the tx queue earlier than needed.
Will look into this...
OK, I've fixed this.
[...]
+ desc->ds = skb->len;
+ desc->dptr = dma_map_single(&ndev->dev, buffer, skb->len,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(&ndev->dev, desc->dptr)) {
+ dev_kfree_skb_any(skb);
+ priv->tx_skb[q][entry] = NULL;
Don't you need to make sure this NULL is properly seen by ravb_tx_free()?
You mean doing this before releasing the spinlock? Or what?
Yes, the locking your transmit function seems to open windows during
which it is possible for the interrupt handler running on another CPU to
mess up with the data you are using here.
Will look into that too...
I have looked into the code and I must admit I don't understand how the
data can be messed up with. ravb_tx_free() only advances 'priv->dirty_tx' and
doesn't go beyond (or change) 'priv->cur_tx' which is used here...
--
Florian
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
