| Message ID | 6672379.kyQ5eFYq8y@wasted.cogentembedded.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Date: Wed, 04 Nov 2015 00:55:13 +0300 > While the ring allocation is done by a single function, sh_eth_ring_init(), > the ring deallocation was split into two functions (almost always called > one after the other) for no good reason. Merge sh_eth_free_dma_buffer() > into sh_eth_ring_free() which allows us to save space not only on the > direct calls of the former function but also on the sh_eth_ring_init()'s > simplified error path... > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 11/5/2015 4:59 AM, David Miller wrote: >> While the ring allocation is done by a single function, sh_eth_ring_init(), >> the ring deallocation was split into two functions (almost always called >> one after the other) for no good reason. Merge sh_eth_free_dma_buffer() >> into sh_eth_ring_free() which allows us to save space not only on the >> direct calls of the former function but also on the sh_eth_ring_init()'s >> simplified error path... >> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > Applied. Hum, I'm seeing both patches in the net.git repo, while they were clearly targeted to net-next.git... Did you really consider these 2 patches fixes? MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Date: Thu, 5 Nov 2015 15:53:24 +0300 > Hello. > > On 11/5/2015 4:59 AM, David Miller wrote: > >>> While the ring allocation is done by a single function, >>> sh_eth_ring_init(), >>> the ring deallocation was split into two functions (almost always >>> called >>> one after the other) for no good reason. Merge >>> sh_eth_free_dma_buffer() >>> into sh_eth_ring_free() which allows us to save space not only on the >>> direct calls of the former function but also on the >>> sh_eth_ring_init()'s >>> simplified error path... >>> >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> >> Applied. > > Hum, I'm seeing both patches in the net.git repo, while they were > clearly targeted to net-next.git... Did you really consider these 2 > patches fixes? It was more work for me to let the patches rot in patchwork until I openned net-next back up than to simply just apply them to net. You guys really make an enormous amount of work and stress for me when you submit net-next patches when I _CLEARLY_ and _EXPLICITLY_ state that the tree is closed right now. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 11/05/2015 07:13 PM, David Miller wrote: >>>> While the ring allocation is done by a single function, >>>> sh_eth_ring_init(), >>>> the ring deallocation was split into two functions (almost always >>>> called >>>> one after the other) for no good reason. Merge >>>> sh_eth_free_dma_buffer() >>>> into sh_eth_ring_free() which allows us to save space not only on the >>>> direct calls of the former function but also on the >>>> sh_eth_ring_init()'s >>>> simplified error path... >>>> >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>> >>> Applied. >> >> Hum, I'm seeing both patches in the net.git repo, while they were >> clearly targeted to net-next.git... Did you really consider these 2 >> patches fixes? > > It was more work for me to let the patches rot in patchwork until I openned > net-next back up than to simply just apply them to net. OK, thank you! > You guys really make an enormous amount of work and stress for me when you > submit net-next patches when I _CLEARLY_ and _EXPLICITLY_ state that the > tree is closed right now. Hmm, I hadn't seen your announcement, else I would have refrained from sending. Will look for it now... MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/05/2015 08:19 PM, Sergei Shtylyov wrote: >> You guys really make an enormous amount of work and stress for me when you >> submit net-next patches when I _CLEARLY_ and _EXPLICITLY_ state that the >> tree is closed right now. > > Hmm, I hadn't seen your announcement, else I would have refrained from > sending. Will look for it now... Nay, still not seeing anything... MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Date: Thu, 5 Nov 2015 20:19:17 +0300 > Hmm, I hadn't seen your announcement, else I would have refrained from > sending. Will look for it now... I really don't know how to better get people's attention than this: http://marc.info/?l=linux-netdev&m=144652382428132&w=2 Do I have to use all CAPS? Flashing LEDs in the Subject? Just tell me, I'll do it... -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/05/2015 09:29 PM, David Miller wrote: >> Hmm, I hadn't seen your announcement, else I would have refrained from >> sending. Will look for it now... > > I really don't know how to better get people's attention than this: > > http://marc.info/?l=linux-netdev&m=144652382428132&w=2 Umm, I'm seeing it now, it's been even marked as read but I for the life of me couldn't remember reading it. Now I'll have to say I'm really sorry for sending out my last 2 or 3 patches, I was under full impression your tree is still open... :-< > Do I have to use all CAPS? Well, perhaps that would have drawn more attention (it has in the past). :-) > Flashing LEDs in the Subject? :-) MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 05, 2015 at 01:29:15PM -0500, David Miller wrote: > From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > Date: Thu, 5 Nov 2015 20:19:17 +0300 > > > Hmm, I hadn't seen your announcement, else I would have refrained from > > sending. Will look for it now... > > I really don't know how to better get people's attention than this: > > http://marc.info/?l=linux-netdev&m=144652382428132&w=2 > > Do I have to use all CAPS? Flashing LEDs in the Subject? > > Just tell me, I'll do it... web page on www.isnetnextopen.com, with a giant word saying YES or NO :) Dave -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: net-next/drivers/net/ethernet/renesas/sh_eth.c =================================================================== --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c +++ net-next/drivers/net/ethernet/renesas/sh_eth.c @@ -1098,7 +1098,7 @@ static struct mdiobb_ops bb_ops = { static void sh_eth_ring_free(struct net_device *ndev) { struct sh_eth_private *mdp = netdev_priv(ndev); - int i; + int ringsize, i; /* Free Rx skb ringbuffer */ if (mdp->rx_skbuff) { @@ -1115,6 +1115,20 @@ static void sh_eth_ring_free(struct net_ } kfree(mdp->tx_skbuff); mdp->tx_skbuff = NULL; + + if (mdp->rx_ring) { + ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring; + dma_free_coherent(NULL, ringsize, mdp->rx_ring, + mdp->rx_desc_dma); + mdp->rx_ring = NULL; + } + + if (mdp->tx_ring) { + ringsize = sizeof(struct sh_eth_txdesc) * mdp->num_tx_ring; + dma_free_coherent(NULL, ringsize, mdp->tx_ring, + mdp->tx_desc_dma); + mdp->tx_ring = NULL; + } } /* format skb and descriptor buffer */ @@ -1220,14 +1234,14 @@ static int sh_eth_ring_init(struct net_d mdp->tx_skbuff = kcalloc(mdp->num_tx_ring, sizeof(*mdp->tx_skbuff), GFP_KERNEL); if (!mdp->tx_skbuff) - goto skb_ring_free; + goto ring_free; /* Allocate all Rx descriptors. */ rx_ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring; mdp->rx_ring = dma_alloc_coherent(NULL, rx_ringsize, &mdp->rx_desc_dma, GFP_KERNEL); if (!mdp->rx_ring) - goto skb_ring_free; + goto ring_free; mdp->dirty_rx = 0; @@ -1236,41 +1250,16 @@ static int sh_eth_ring_init(struct net_d mdp->tx_ring = dma_alloc_coherent(NULL, tx_ringsize, &mdp->tx_desc_dma, GFP_KERNEL); if (!mdp->tx_ring) - goto desc_ring_free; + goto ring_free; return 0; -desc_ring_free: - /* free DMA buffer */ - dma_free_coherent(NULL, rx_ringsize, mdp->rx_ring, mdp->rx_desc_dma); - -skb_ring_free: - /* Free Rx and Tx skb ring buffer */ +ring_free: + /* Free Rx and Tx skb ring buffer and DMA buffer */ sh_eth_ring_free(ndev); - mdp->tx_ring = NULL; - mdp->rx_ring = NULL; return -ENOMEM; } -static void sh_eth_free_dma_buffer(struct sh_eth_private *mdp) -{ - int ringsize; - - if (mdp->rx_ring) { - ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring; - dma_free_coherent(NULL, ringsize, mdp->rx_ring, - mdp->rx_desc_dma); - mdp->rx_ring = NULL; - } - - if (mdp->tx_ring) { - ringsize = sizeof(struct sh_eth_txdesc) * mdp->num_tx_ring; - dma_free_coherent(NULL, ringsize, mdp->tx_ring, - mdp->tx_desc_dma); - mdp->tx_ring = NULL; - } -} - static int sh_eth_dev_init(struct net_device *ndev, bool start) { int ret = 0; @@ -2231,10 +2220,8 @@ static int sh_eth_set_ringparam(struct n sh_eth_dev_exit(ndev); - /* Free all the skbuffs in the Rx queue. */ + /* Free all the skbuffs in the Rx queue and the DMA buffers. */ sh_eth_ring_free(ndev); - /* Free DMA buffer */ - sh_eth_free_dma_buffer(mdp); } /* Set new parameters */ @@ -2479,12 +2466,9 @@ static int sh_eth_close(struct net_devic free_irq(ndev->irq, ndev); - /* Free all the skbuffs in the Rx queue. */ + /* Free all the skbuffs in the Rx queue and the DMA buffer. */ sh_eth_ring_free(ndev); - /* free DMA buffer */ - sh_eth_free_dma_buffer(mdp); - pm_runtime_put_sync(&mdp->pdev->dev); mdp->is_opened = 0;
While the ring allocation is done by a single function, sh_eth_ring_init(), the ring deallocation was split into two functions (almost always called one after the other) for no good reason. Merge sh_eth_free_dma_buffer() into sh_eth_ring_free() which allows us to save space not only on the direct calls of the former function but also on the sh_eth_ring_init()'s simplified error path... Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- The patch is against DaveM's 'net-next.git' repo plus the patch just posted. drivers/net/ethernet/renesas/sh_eth.c | 60 ++++++++++++---------------------- 1 file changed, 22 insertions(+), 38 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html