* [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() [not found] ` <1645988.6VIRltF7C5@avalon> @ 2016-04-22 1:50 ` Kuninori Morimoto 2016-05-02 9:37 ` Vinod Koul 2016-05-14 7:57 ` Vinod Koul 2016-05-30 0:41 ` [PATCH v2] " Kuninori Morimoto 1 sibling, 2 replies; 11+ messages in thread From: Kuninori Morimoto @ 2016-04-22 1:50 UTC (permalink / raw) To: Vinod Koul, Geert Uytterhoeven, Laurent Pinchart Cc: sakato, OSD2 ML, dmaengine, linux-kernel From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Current rcar_dmac_desc_put() is using list_add_tail() in order to push used descriptor to list of free descriptors, and next DMA transfer try to reuse it from this list. But because it is using *_tail(), this reuse effect can't be obtained without using all of them. For a longer-term solution, we should allocate hardware descriptors using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. This patch uses list_add() instead of list_add_tail() for short-term solution. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- drivers/dma/sh/rcar-dmac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index 02b86c6..616c63a 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -519,7 +519,7 @@ static void rcar_dmac_desc_put(struct rcar_dmac_chan *chan, spin_lock_irqsave(&chan->lock, flags); list_splice_tail_init(&desc->chunks, &chan->desc.chunks_free); - list_add_tail(&desc->node, &chan->desc.free); + list_add(&desc->node, &chan->desc.free); spin_unlock_irqrestore(&chan->lock, flags); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-04-22 1:50 ` [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() Kuninori Morimoto @ 2016-05-02 9:37 ` Vinod Koul 2016-05-09 20:49 ` Laurent Pinchart 2016-05-14 7:57 ` Vinod Koul 1 sibling, 1 reply; 11+ messages in thread From: Vinod Koul @ 2016-05-02 9:37 UTC (permalink / raw) To: Kuninori Morimoto Cc: Geert Uytterhoeven, Laurent Pinchart, sakato, OSD2 ML, dmaengine, linux-kernel On Fri, Apr 22, 2016 at 01:50:04AM +0000, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Current rcar_dmac_desc_put() is using list_add_tail() in order to > push used descriptor to list of free descriptors, and next DMA transfer > try to reuse it from this list. But because it is using *_tail(), > this reuse effect can't be obtained without using all of them. > For a longer-term solution, we should allocate hardware descriptors > using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. > This patch uses list_add() instead of list_add_tail() for short-term > solution. So how does reuse case help by not moving the descriptor to tail. Also you are not reusing descriptor, you are reusing a descriptor memory, these are two different things. Lastly how does this help? Something doesn't seem right > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > drivers/dma/sh/rcar-dmac.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > index 02b86c6..616c63a 100644 > --- a/drivers/dma/sh/rcar-dmac.c > +++ b/drivers/dma/sh/rcar-dmac.c > @@ -519,7 +519,7 @@ static void rcar_dmac_desc_put(struct rcar_dmac_chan *chan, > > spin_lock_irqsave(&chan->lock, flags); > list_splice_tail_init(&desc->chunks, &chan->desc.chunks_free); > - list_add_tail(&desc->node, &chan->desc.free); > + list_add(&desc->node, &chan->desc.free); > spin_unlock_irqrestore(&chan->lock, flags); > } > -- ~Vinod ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-05-02 9:37 ` Vinod Koul @ 2016-05-09 20:49 ` Laurent Pinchart 2016-05-11 3:28 ` Kuninori Morimoto 0 siblings, 1 reply; 11+ messages in thread From: Laurent Pinchart @ 2016-05-09 20:49 UTC (permalink / raw) To: Vinod Koul Cc: Kuninori Morimoto, Geert Uytterhoeven, Laurent Pinchart, sakato, OSD2 ML, dmaengine, linux-kernel Hi Vinod, On Monday 02 May 2016 15:07:12 Vinod Koul wrote: > On Fri, Apr 22, 2016 at 01:50:04AM +0000, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > Current rcar_dmac_desc_put() is using list_add_tail() in order to > > push used descriptor to list of free descriptors, and next DMA transfer > > try to reuse it from this list. But because it is using *_tail(), > > this reuse effect can't be obtained without using all of them. > > For a longer-term solution, we should allocate hardware descriptors > > using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. > > This patch uses list_add() instead of list_add_tail() for short-term > > solution. > > So how does reuse case help by not moving the descriptor to tail. > > Also you are not reusing descriptor, you are reusing a descriptor memory, > these are two different things. > > Lastly how does this help? Something doesn't seem right For each descriptor, in addition to the memory used by the descriptors structure itself, the driver allocates a list of chunks as well as a buffer for hardware descriptors. Descriptors themselves are preallocated, and allocation of the chunks and buffer is performed the first time the descriptor is used. The memory isn't freed when the transfer is completed, as the chunks and buffer will be needed again when the descriptor is reused internally, so the driver keeps the memory around. If only a few descriptors are used concurrently, the current list_add_tail() implementation will result in all preallocated descriptors being used before going back to the first one, and will thus allocate chunks and a buffer for all preallocated descriptors. Using list_add() will put the complete descriptor at the head of the list of available descriptors, so the next transfer will be more likely to reuse a descriptor that already has associated memory instead of one that has never been used before. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > --- > > > > drivers/dma/sh/rcar-dmac.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > > index 02b86c6..616c63a 100644 > > --- a/drivers/dma/sh/rcar-dmac.c > > +++ b/drivers/dma/sh/rcar-dmac.c > > @@ -519,7 +519,7 @@ static void rcar_dmac_desc_put(struct rcar_dmac_chan > > *chan,> > > spin_lock_irqsave(&chan->lock, flags); > > list_splice_tail_init(&desc->chunks, &chan->desc.chunks_free); > > > > - list_add_tail(&desc->node, &chan->desc.free); > > + list_add(&desc->node, &chan->desc.free); > > > > spin_unlock_irqrestore(&chan->lock, flags); > > > > } -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-05-09 20:49 ` Laurent Pinchart @ 2016-05-11 3:28 ` Kuninori Morimoto 2016-05-14 8:11 ` Vinod Koul 0 siblings, 1 reply; 11+ messages in thread From: Kuninori Morimoto @ 2016-05-11 3:28 UTC (permalink / raw) To: Laurent Pinchart Cc: Vinod Koul, Geert Uytterhoeven, Laurent Pinchart, sakato, OSD2 ML, dmaengine, linux-kernel Hi Laurent, Vinod, > > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > > > Current rcar_dmac_desc_put() is using list_add_tail() in order to > > > push used descriptor to list of free descriptors, and next DMA transfer > > > try to reuse it from this list. But because it is using *_tail(), > > > this reuse effect can't be obtained without using all of them. > > > For a longer-term solution, we should allocate hardware descriptors > > > using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. > > > This patch uses list_add() instead of list_add_tail() for short-term > > > solution. > > > > So how does reuse case help by not moving the descriptor to tail. > > > > Also you are not reusing descriptor, you are reusing a descriptor memory, > > these are two different things. > > > > Lastly how does this help? Something doesn't seem right > > For each descriptor, in addition to the memory used by the descriptors > structure itself, the driver allocates a list of chunks as well as a buffer > for hardware descriptors. Descriptors themselves are preallocated, and > allocation of the chunks and buffer is performed the first time the descriptor > is used. The memory isn't freed when the transfer is completed, as the chunks > and buffer will be needed again when the descriptor is reused internally, so > the driver keeps the memory around. > > If only a few descriptors are used concurrently, the current list_add_tail() > implementation will result in all preallocated descriptors being used before > going back to the first one, and will thus allocate chunks and a buffer for > all preallocated descriptors. Using list_add() will put the complete > descriptor at the head of the list of available descriptors, so the next > transfer will be more likely to reuse a descriptor that already has associated > memory instead of one that has never been used before. Laurent, thank you for your help Vinod, does above clear for you ? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-05-11 3:28 ` Kuninori Morimoto @ 2016-05-14 8:11 ` Vinod Koul 0 siblings, 0 replies; 11+ messages in thread From: Vinod Koul @ 2016-05-14 8:11 UTC (permalink / raw) To: Kuninori Morimoto Cc: Laurent Pinchart, Geert Uytterhoeven, Laurent Pinchart, sakato, OSD2 ML, dmaengine, linux-kernel On Wed, May 11, 2016 at 03:28:04AM +0000, Kuninori Morimoto wrote: > > Hi Laurent, Vinod, > > > > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > > > > > Current rcar_dmac_desc_put() is using list_add_tail() in order to > > > > push used descriptor to list of free descriptors, and next DMA transfer > > > > try to reuse it from this list. But because it is using *_tail(), > > > > this reuse effect can't be obtained without using all of them. > > > > For a longer-term solution, we should allocate hardware descriptors > > > > using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. > > > > This patch uses list_add() instead of list_add_tail() for short-term > > > > solution. > > > > > > So how does reuse case help by not moving the descriptor to tail. > > > > > > Also you are not reusing descriptor, you are reusing a descriptor memory, > > > these are two different things. > > > > > > Lastly how does this help? Something doesn't seem right > > > > For each descriptor, in addition to the memory used by the descriptors > > structure itself, the driver allocates a list of chunks as well as a buffer > > for hardware descriptors. Descriptors themselves are preallocated, and > > allocation of the chunks and buffer is performed the first time the descriptor > > is used. The memory isn't freed when the transfer is completed, as the chunks > > and buffer will be needed again when the descriptor is reused internally, so > > the driver keeps the memory around. > > > > If only a few descriptors are used concurrently, the current list_add_tail() > > implementation will result in all preallocated descriptors being used before > > going back to the first one, and will thus allocate chunks and a buffer for > > all preallocated descriptors. Using list_add() will put the complete > > descriptor at the head of the list of available descriptors, so the next > > transfer will be more likely to reuse a descriptor that already has associated > > memory instead of one that has never been used before. > > Laurent, thank you for your help > Vinod, does above clear for you ? Yes makese sense now. But please add these details in the changelog. This helps people know why a line was modified down the line -- ~Vinod ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-04-22 1:50 ` [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() Kuninori Morimoto 2016-05-02 9:37 ` Vinod Koul @ 2016-05-14 7:57 ` Vinod Koul 2016-05-24 9:50 ` Laurent Pinchart 1 sibling, 1 reply; 11+ messages in thread From: Vinod Koul @ 2016-05-14 7:57 UTC (permalink / raw) To: Kuninori Morimoto Cc: Geert Uytterhoeven, Laurent Pinchart, sakato, OSD2 ML, dmaengine, linux-kernel On Fri, Apr 22, 2016 at 01:50:04AM +0000, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Current rcar_dmac_desc_put() is using list_add_tail() in order to > push used descriptor to list of free descriptors, and next DMA transfer > try to reuse it from this list. But because it is using *_tail(), > this reuse effect can't be obtained without using all of them. > For a longer-term solution, we should allocate hardware descriptors > using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. > This patch uses list_add() instead of list_add_tail() for short-term > solution. Applied, thanks -- ~Vinod ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-05-14 7:57 ` Vinod Koul @ 2016-05-24 9:50 ` Laurent Pinchart 2016-05-26 15:34 ` Vinod Koul 0 siblings, 1 reply; 11+ messages in thread From: Laurent Pinchart @ 2016-05-24 9:50 UTC (permalink / raw) To: Vinod Koul Cc: Kuninori Morimoto, Geert Uytterhoeven, Laurent Pinchart, sakato, OSD2 ML, dmaengine, linux-kernel Hi Vinod, On Saturday 14 May 2016 13:27:31 Vinod Koul wrote: > On Fri, Apr 22, 2016 at 01:50:04AM +0000, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > Current rcar_dmac_desc_put() is using list_add_tail() in order to > > push used descriptor to list of free descriptors, and next DMA transfer > > try to reuse it from this list. But because it is using *_tail(), > > this reuse effect can't be obtained without using all of them. > > For a longer-term solution, we should allocate hardware descriptors > > using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. > > This patch uses list_add() instead of list_add_tail() for short-term > > solution. > > Applied, thanks Thanks, but where did you apply it to ? I can't find it in your tree. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-05-24 9:50 ` Laurent Pinchart @ 2016-05-26 15:34 ` Vinod Koul 2016-05-30 0:34 ` Kuninori Morimoto 0 siblings, 1 reply; 11+ messages in thread From: Vinod Koul @ 2016-05-26 15:34 UTC (permalink / raw) To: Laurent Pinchart Cc: Kuninori Morimoto, Geert Uytterhoeven, Laurent Pinchart, sakato, OSD2 ML, dmaengine, linux-kernel On Tue, May 24, 2016 at 12:50:28PM +0300, Laurent Pinchart wrote: Hey Laurent, > On Saturday 14 May 2016 13:27:31 Vinod Koul wrote: > > On Fri, Apr 22, 2016 at 01:50:04AM +0000, Kuninori Morimoto wrote: > > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > > > Current rcar_dmac_desc_put() is using list_add_tail() in order to > > > push used descriptor to list of free descriptors, and next DMA transfer > > > try to reuse it from this list. But because it is using *_tail(), > > > this reuse effect can't be obtained without using all of them. > > > For a longer-term solution, we should allocate hardware descriptors > > > using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. > > > This patch uses list_add() instead of list_add_tail() for short-term > > > solution. > > > > Applied, thanks > > Thanks, but where did you apply it to ? I can't find it in your tree. I changed my mind after this and felt that we should document above in the changelog as well and replied to Kuninori few moments after this [1] Sorry if that wasn't clear that am not applying this. Please resend it with update changelog [1]: http://www.spinics.net/lists/dmaengine/msg09585.html -- ~Vinod ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-05-26 15:34 ` Vinod Koul @ 2016-05-30 0:34 ` Kuninori Morimoto 0 siblings, 0 replies; 11+ messages in thread From: Kuninori Morimoto @ 2016-05-30 0:34 UTC (permalink / raw) To: Vinod Koul Cc: Laurent Pinchart, Geert Uytterhoeven, Laurent Pinchart, sakato, OSD2 ML, dmaengine, linux-kernel Hi Vinod > > > > Current rcar_dmac_desc_put() is using list_add_tail() in order to > > > > push used descriptor to list of free descriptors, and next DMA transfer > > > > try to reuse it from this list. But because it is using *_tail(), > > > > this reuse effect can't be obtained without using all of them. > > > > For a longer-term solution, we should allocate hardware descriptors > > > > using GFP_KERNEL instead of GFP_NOWAIT, but it is difficult today. > > > > This patch uses list_add() instead of list_add_tail() for short-term > > > > solution. > > > > > > Applied, thanks > > > > Thanks, but where did you apply it to ? I can't find it in your tree. > > I changed my mind after this and felt that we should document above > in the changelog as well and replied to Kuninori few moments after this [1] > > Sorry if that wasn't clear that am not applying this. > > Please resend it with update changelog > > [1]: http://www.spinics.net/lists/dmaengine/msg09585.html OK, will do Best regards --- Kuninori Morimoto ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() [not found] ` <1645988.6VIRltF7C5@avalon> 2016-04-22 1:50 ` [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() Kuninori Morimoto @ 2016-05-30 0:41 ` Kuninori Morimoto 2016-05-30 3:42 ` Vinod Koul 1 sibling, 1 reply; 11+ messages in thread From: Kuninori Morimoto @ 2016-05-30 0:41 UTC (permalink / raw) To: Vinod Koul, Geert Uytterhoeven, Laurent Pinchart Cc: dmaengine, sakato, linux-kernel, OSD2 ML From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> For each descriptor, in addition to the memory used by the descriptors structure itself, the driver allocates a list of chunks as well as a buffer for hardware descriptors. Descriptors themselves are preallocated, and allocation of the chunks and buffer is performed the first time the descriptor is used. The memory isn't freed when the transfer is completed, as the chunks and buffer will be needed again when the descriptor is reused internally, so the driver keeps the memory around. If only a few descriptors are used concurrently, the current list_add_tail() implementation will result in all preallocated descriptors being used before going back to the first one, and will thus allocate chunks and a buffer for all preallocated descriptors. Using list_add() will put the complete descriptor at the head of the list of available descriptors, so the next transfer will be more likely to reuse a descriptor that already has associated memory instead of one that has never been used before. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- v1 -> v2 - used Laurent's explain on log drivers/dma/sh/rcar-dmac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index 02b86c6..616c63a 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -519,7 +519,7 @@ static void rcar_dmac_desc_put(struct rcar_dmac_chan *chan, spin_lock_irqsave(&chan->lock, flags); list_splice_tail_init(&desc->chunks, &chan->desc.chunks_free); - list_add_tail(&desc->node, &chan->desc.free); + list_add(&desc->node, &chan->desc.free); spin_unlock_irqrestore(&chan->lock, flags); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() 2016-05-30 0:41 ` [PATCH v2] " Kuninori Morimoto @ 2016-05-30 3:42 ` Vinod Koul 0 siblings, 0 replies; 11+ messages in thread From: Vinod Koul @ 2016-05-30 3:42 UTC (permalink / raw) To: Kuninori Morimoto Cc: Geert Uytterhoeven, Laurent Pinchart, dmaengine, sakato, linux-kernel, OSD2 ML On Mon, May 30, 2016 at 12:41:48AM +0000, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > For each descriptor, in addition to the memory used by the descriptors > structure itself, the driver allocates a list of chunks as well as a > buffer for hardware descriptors. Descriptors themselves are preallocated, > and allocation of the chunks and buffer is performed the first time the > descriptor is used. The memory isn't freed when the transfer is completed, > as the chunks and buffer will be needed again when the descriptor is > reused internally, so the driver keeps the memory around. > > If only a few descriptors are used concurrently, the current > list_add_tail() implementation will result in all preallocated descriptors > being used before going back to the first one, and will thus allocate > chunks and a buffer for all preallocated descriptors. Using list_add() > will put the complete descriptor at the head of the list of available > descriptors, so the next transfer will be more likely to reuse a > descriptor that already has associated memory instead of one that has > never been used before. Applied after fixing subsystem name -- ~Vinod ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-05-30 3:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <redmine.journal-676557.20160325074450.6aeff6eaa5fae69d@dm.renesas.com>
[not found] ` <8760w7xq3n.wl%kuninori.morimoto.gx@renesas.com>
[not found] ` <874mbrxnft.wl%kuninori.morimoto.gx@renesas.com>
[not found] ` <1645988.6VIRltF7C5@avalon>
2016-04-22 1:50 ` [PATCH] dma: rcar-dmac: use list_add() on rcar_dmac_desc_put() Kuninori Morimoto
2016-05-02 9:37 ` Vinod Koul
2016-05-09 20:49 ` Laurent Pinchart
2016-05-11 3:28 ` Kuninori Morimoto
2016-05-14 8:11 ` Vinod Koul
2016-05-14 7:57 ` Vinod Koul
2016-05-24 9:50 ` Laurent Pinchart
2016-05-26 15:34 ` Vinod Koul
2016-05-30 0:34 ` Kuninori Morimoto
2016-05-30 0:41 ` [PATCH v2] " Kuninori Morimoto
2016-05-30 3:42 ` Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).