Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dmaengine: pl08x: be fair when re-assigning physical channel
@ 2019-03-01 16:18 Jean-Nicolas Graux
  2019-03-04 10:00 ` Linus Walleij
  0 siblings, 1 reply; 8+ messages in thread
From: Jean-Nicolas Graux @ 2019-03-01 16:18 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-arm-kernel

Current way we find a waiting virtual channel for the next transfer
at the time one physical channel becomes free is not really fair.

More in details, in case there is more than one channel waiting at a time,
by just going through the arrays of memcpy and slave channels and stopping
as soon as state match waiting state, channels with high indexes can be
penalized.

Whenever dma engine is substantially overloaded so that we constantly
get several channels waiting, channels with highest indexes might not
be served for a substantial time which in the worse case, might hang
task that wait for dma transfer to complete.

This patch makes physical channel re-assignment more fair by storing
time in jiffies when a channel is put in waiting state. Whenever a
physical channel has to be re-assigned, this time is used to select
channel that is waiting for the longest time.

Signed-off-by: Jean-Nicolas Graux <jean-nicolas.graux@st.com>
---
 drivers/dma/amba-pl08x.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index fc8c2ba..4da830d 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -254,6 +254,7 @@ enum pl08x_dma_chan_state {
  * @slave: whether this channel is a device (slave) or for memcpy
  * @signal: the physical DMA request signal which this channel is using
  * @mux_use: count of descriptors using this DMA request signal setting
+ * @waiting_at: time in jiffies when this channel moved to waiting state
  */
 struct pl08x_dma_chan {
 	struct virt_dma_chan vc;
@@ -267,6 +268,7 @@ struct pl08x_dma_chan {
 	bool slave;
 	int signal;
 	unsigned mux_use;
+	unsigned long waiting_at;
 };
 
 /**
@@ -875,6 +877,7 @@ static void pl08x_phy_alloc_and_start(struct pl08x_dma_chan *plchan)
 	if (!ch) {
 		dev_dbg(&pl08x->adev->dev, "no physical channel available for xfer on %s\n", plchan->name);
 		plchan->state = PL08X_CHAN_WAITING;
+		plchan->waiting_at = jiffies;
 		return;
 	}
 
@@ -913,22 +916,25 @@ static void pl08x_phy_free(struct pl08x_dma_chan *plchan)
 {
 	struct pl08x_driver_data *pl08x = plchan->host;
 	struct pl08x_dma_chan *p, *next;
-
+	unsigned long waiting_at;
  retry:
 	next = NULL;
+	waiting_at = jiffies;
 
 	/* Find a waiting virtual channel for the next transfer. */
 	list_for_each_entry(p, &pl08x->memcpy.channels, vc.chan.device_node)
-		if (p->state == PL08X_CHAN_WAITING) {
+		if (p->state == PL08X_CHAN_WAITING &&
+		    p->waiting_at <= waiting_at) {
 			next = p;
-			break;
+			waiting_at = p->waiting_at;
 		}
 
 	if (!next && pl08x->has_slave) {
 		list_for_each_entry(p, &pl08x->slave.channels, vc.chan.device_node)
-			if (p->state == PL08X_CHAN_WAITING) {
+			if (p->state == PL08X_CHAN_WAITING &&
+			    p->waiting_at <= waiting_at) {
 				next = p;
-				break;
+				waiting_at = p->waiting_at;
 			}
 	}
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH] dmaengine: pl08x: be fair when re-assigning physical channel
@ 2019-03-04 15:03 Jean-Nicolas Graux
  2019-03-25  5:13 ` Vinod Koul
  0 siblings, 1 reply; 8+ messages in thread
From: Jean-Nicolas Graux @ 2019-03-04 15:03 UTC (permalink / raw)
  To: Linus Walleij, Vinod Koul; +Cc: dmaengine, linux-arm-kernel

Current way we find a waiting virtual channel for the next transfer
at the time one physical channel becomes free is not really fair.

More in details, in case there is more than one channel waiting at a time,
by just going through the arrays of memcpy and slave channels and stopping
as soon as state match waiting state, channels with high indexes can be
penalized.

Whenever dma engine is substantially overloaded so that we constantly
get several channels waiting, channels with highest indexes might not
be served for a substantial time which in the worse case, might hang
task that wait for dma transfer to complete.

This patch makes physical channel re-assignment more fair by storing
time in jiffies when a channel is put in waiting state. Whenever a
physical channel has to be re-assigned, this time is used to select
channel that is waiting for the longest time.

Signed-off-by: Jean-Nicolas Graux <jean-nicolas.graux@st.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Nicolas Guion <nicolas.guion@st.com>
---
 drivers/dma/amba-pl08x.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index fc8c2ba..8cfc753 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -254,6 +254,7 @@ enum pl08x_dma_chan_state {
  * @slave: whether this channel is a device (slave) or for memcpy
  * @signal: the physical DMA request signal which this channel is using
  * @mux_use: count of descriptors using this DMA request signal setting
+ * @waiting_at: time in jiffies when this channel moved to waiting state
  */
 struct pl08x_dma_chan {
 	struct virt_dma_chan vc;
@@ -267,6 +268,7 @@ struct pl08x_dma_chan {
 	bool slave;
 	int signal;
 	unsigned mux_use;
+	unsigned long waiting_at;
 };
 
 /**
@@ -875,6 +877,7 @@ static void pl08x_phy_alloc_and_start(struct pl08x_dma_chan *plchan)
 	if (!ch) {
 		dev_dbg(&pl08x->adev->dev, "no physical channel available for xfer on %s\n", plchan->name);
 		plchan->state = PL08X_CHAN_WAITING;
+		plchan->waiting_at = jiffies;
 		return;
 	}
 
@@ -913,22 +916,29 @@ static void pl08x_phy_free(struct pl08x_dma_chan *plchan)
 {
 	struct pl08x_driver_data *pl08x = plchan->host;
 	struct pl08x_dma_chan *p, *next;
-
+	unsigned long waiting_at;
  retry:
 	next = NULL;
+	waiting_at = jiffies;
 
-	/* Find a waiting virtual channel for the next transfer. */
+	/*
+	 * Find a waiting virtual channel for the next transfer.
+	 * To be fair, time when each channel reached waiting state is compared
+	 * to select channel that is waiting for the longest time.
+	 */
 	list_for_each_entry(p, &pl08x->memcpy.channels, vc.chan.device_node)
-		if (p->state == PL08X_CHAN_WAITING) {
+		if (p->state == PL08X_CHAN_WAITING &&
+		    p->waiting_at <= waiting_at) {
 			next = p;
-			break;
+			waiting_at = p->waiting_at;
 		}
 
 	if (!next && pl08x->has_slave) {
 		list_for_each_entry(p, &pl08x->slave.channels, vc.chan.device_node)
-			if (p->state == PL08X_CHAN_WAITING) {
+			if (p->state == PL08X_CHAN_WAITING &&
+			    p->waiting_at <= waiting_at) {
 				next = p;
-				break;
+				waiting_at = p->waiting_at;
 			}
 	}
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-25  8:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-01 16:18 [PATCH] dmaengine: pl08x: be fair when re-assigning physical channel Jean-Nicolas Graux
2019-03-04 10:00 ` Linus Walleij
2019-03-04 10:37   ` Jean Nicolas GRAUX
2019-03-04 12:05     ` Linus Walleij
2019-03-04 13:59       ` Jean Nicolas GRAUX
  -- strict thread matches above, loose matches on Subject: below --
2019-03-04 15:03 Jean-Nicolas Graux
2019-03-25  5:13 ` Vinod Koul
2019-03-25  8:15   ` Jean Nicolas GRAUX

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox