| Message ID | 20130829201151.GJ8764@redacted.bos.redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On 08/29/2013 03:11 PM, Kyle McMartin wrote: > Noticed while debugging another issue that DMA debug turned up, looks > like commit ef468d23 missed a case when switching to bufsz instead of > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly > suspect since we're not tracking the size, but trusting the hardware > between map and unmap...) Please see my patch series of fixes: http://www.spinics.net/lists/netdev/msg248076.html > Signed-off-by: Kyle McMartin <kyle@redhat.com> > > --- a/drivers/net/ethernet/calxeda/xgmac.c > +++ b/drivers/net/ethernet/calxeda/xgmac.c > @@ -686,7 +686,7 @@ static void xgmac_rx_refill(struct xgmac_priv *priv) > priv->rx_skbuff[entry] = skb; > paddr = dma_map_single(priv->device, skb->data, > bufsz, DMA_FROM_DEVICE); > - desc_set_buf_addr(p, paddr, priv->dma_buf_sz); > + desc_set_buf_addr(p, paddr, bufsz); This is not right because the h/w wants the size to include the 2 bytes skipped at the beginning for NET_IP_ALIGN. The h/w may also corrupt 1-7 bytes of data at the head or tail if the frame is not 8 byte aligned and padded. We are relying that skb allocations are aligned. Rob -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 29, 2013 at 03:24:30PM -0500, Rob Herring wrote: > On 08/29/2013 03:11 PM, Kyle McMartin wrote: > > Noticed while debugging another issue that DMA debug turned up, looks > > like commit ef468d23 missed a case when switching to bufsz instead of > > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly > > suspect since we're not tracking the size, but trusting the hardware > > between map and unmap...) > > Please see my patch series of fixes: > > http://www.spinics.net/lists/netdev/msg248076.html > Hah, thanks. Yeah, I wondered wtf was up with get_buf_len, but figured the hardware must have been doing something subtle. ;-) I'll apply these to our Fedora tree and pass them along for testing. Thanks Rob. --Kyle -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 08/30/2013 12:11 AM, Kyle McMartin wrote: > Noticed while debugging another issue that DMA debug turned up, looks > like commit ef468d23 missed a case when switching to bufsz instead of Please also specify that commit's summary line in parens. > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly > suspect since we're not tracking the size, but trusting the hardware > between map and unmap...) > Signed-off-by: Kyle McMartin <kyle@redhat.com> WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Kyle McMartin <kyle@redhat.com> Date: Thu, 29 Aug 2013 16:11:51 -0400 > Noticed while debugging another issue that DMA debug turned up, looks > like commit ef468d23 missed a case when switching to bufsz instead of > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly > suspect since we're not tracking the size, but trusting the hardware > between map and unmap...) > > Signed-off-by: Kyle McMartin <kyle@redhat.com> Applied and queued up for -stable, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 29, 2013 at 04:34:37PM -0400, David Miller wrote: > From: Kyle McMartin <kyle@redhat.com> > Date: Thu, 29 Aug 2013 16:11:51 -0400 > > > Noticed while debugging another issue that DMA debug turned up, looks > > like commit ef468d23 missed a case when switching to bufsz instead of > > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly > > suspect since we're not tracking the size, but trusting the hardware > > between map and unmap...) > > > > Signed-off-by: Kyle McMartin <kyle@redhat.com> > > Applied and queued up for -stable, thanks. Heh, too fast David, Rob nak'd it. :) Sorry about that. --Kyle -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Rob Herring <rob.herring@calxeda.com> Date: Thu, 29 Aug 2013 15:24:30 -0500 > On 08/29/2013 03:11 PM, Kyle McMartin wrote: >> Noticed while debugging another issue that DMA debug turned up, looks >> like commit ef468d23 missed a case when switching to bufsz instead of >> priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly >> suspect since we're not tracking the size, but trusting the hardware >> between map and unmap...) > > Please see my patch series of fixes: > > http://www.spinics.net/lists/netdev/msg248076.html Which need to be resubmitted with feedback intergrated. I knew your series existed, but Kyle submitted a fix in the correct format meanwhile so I applied it. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Kyle McMartin <kmcmarti@redhat.com> Date: Thu, 29 Aug 2013 16:35:23 -0400 > On Thu, Aug 29, 2013 at 04:34:37PM -0400, David Miller wrote: >> From: Kyle McMartin <kyle@redhat.com> >> Date: Thu, 29 Aug 2013 16:11:51 -0400 >> >> > Noticed while debugging another issue that DMA debug turned up, looks >> > like commit ef468d23 missed a case when switching to bufsz instead of >> > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly >> > suspect since we're not tracking the size, but trusting the hardware >> > between map and unmap...) >> > >> > Signed-off-by: Kyle McMartin <kyle@redhat.com> >> >> Applied and queued up for -stable, thanks. > > Heh, too fast David, Rob nak'd it. :) > > Sorry about that. If Rob had resubmitted his series in a timely manner this wouldn't have happened. I reverted. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/29/2013 03:38 PM, David Miller wrote: > From: Kyle McMartin <kmcmarti@redhat.com> > Date: Thu, 29 Aug 2013 16:35:23 -0400 > >> On Thu, Aug 29, 2013 at 04:34:37PM -0400, David Miller wrote: >>> From: Kyle McMartin <kyle@redhat.com> >>> Date: Thu, 29 Aug 2013 16:11:51 -0400 >>> >>>> Noticed while debugging another issue that DMA debug turned up, looks >>>> like commit ef468d23 missed a case when switching to bufsz instead of >>>> priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly >>>> suspect since we're not tracking the size, but trusting the hardware >>>> between map and unmap...) >>>> >>>> Signed-off-by: Kyle McMartin <kyle@redhat.com> >>> >>> Applied and queued up for -stable, thanks. >> >> Heh, too fast David, Rob nak'd it. :) >> >> Sorry about that. > > If Rob had resubmitted his series in a timely manner this wouldn't > have happened. I was giving them more time hoping to have some feedback from Lennert. Anyways, I've just resubmitted v2 which addresses Ben's comments. Rob -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/net/ethernet/calxeda/xgmac.c +++ b/drivers/net/ethernet/calxeda/xgmac.c @@ -686,7 +686,7 @@ static void xgmac_rx_refill(struct xgmac_priv *priv) priv->rx_skbuff[entry] = skb; paddr = dma_map_single(priv->device, skb->data, bufsz, DMA_FROM_DEVICE); - desc_set_buf_addr(p, paddr, priv->dma_buf_sz); + desc_set_buf_addr(p, paddr, bufsz); } netdev_dbg(priv->dev, "rx ring: head %d, tail %d\n",
Noticed while debugging another issue that DMA debug turned up, looks like commit ef468d23 missed a case when switching to bufsz instead of priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly suspect since we're not tracking the size, but trusting the hardware between map and unmap...) Signed-off-by: Kyle McMartin <kyle@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html