All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: tiffany.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org,
	djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org
Subject: [PATCH] iommu/dma: Map scatterlists more parsimoniously
Date: Wed, 11 Nov 2015 14:54:16 +0000	[thread overview]
Message-ID: <104b4682056bc5b4aa0afadd39196c03fc06555d.1447249098.git.robin.murphy@arm.com> (raw)

Whilst blindly assuming the worst case for segment boundaries and
aligning every segment individually is safe from the point of view
of respecting the device's parameters, it is also undeniably a waste
of IOVA space. Futhermore, the knock-on effects of more pages than
necessary being exposed to device access, additional overhead in page
table updates and TLB invalidations, etc., are even more undesirable.

Improve matters by taking the actual boundary mask into account to
actively detect the cases in which we really do need to adjust a
segment, and avoid wasting space in the remainder.

Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---

Hi all,

I've given this some brief testing on Juno with USB and (via magic
PCI hacks) SATA to confirm that all the +1s and -1s at least seem to
be in the right places, so I'm throwing it out now for a head-start on
checking whether it also helps the media folks with the v4l portability
issues they're up against (I'm confident it should). If all goes well I
figure I'll repost next week based on rc1 instead of some random local
development commit.

Robin.

 drivers/iommu/dma-iommu.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 3a20db4..821ebc4 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -441,6 +441,7 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	struct scatterlist *s, *prev = NULL;
 	dma_addr_t dma_addr;
 	size_t iova_len = 0;
+	unsigned long mask = dma_get_seg_boundary(dev);
 	int i;
 
 	/*
@@ -460,17 +461,19 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
 		s->length = s_length;
 
 		/*
-		 * The simple way to avoid the rare case of a segment
-		 * crossing the boundary mask is to pad the previous one
-		 * to end at a naturally-aligned IOVA for this one's size,
-		 * at the cost of potentially over-allocating a little.
+		 * With a single size-aligned IOVA allocation, no segment risks
+		 * crossing the boundary mask unless the total size exceeds
+		 * the mask itself. The simple way to maintain alignment when
+		 * that does happen is to pad the previous segment to end at the
+		 * next boundary, at the cost of over-allocating a little.
 		 */
 		if (prev) {
-			size_t pad_len = roundup_pow_of_two(s_length);
+			size_t pad_len = (mask - iova_len + 1) & mask;
 
-			pad_len = (pad_len - iova_len) & (pad_len - 1);
-			prev->length += pad_len;
-			iova_len += pad_len;
+			if (pad_len && pad_len < s_length - 1) {
+				prev->length += pad_len;
+				iova_len += pad_len;
+			}
 		}
 
 		iova_len += s_length;
-- 
1.9.1

WARNING: multiple messages have this Message-ID (diff)
From: robin.murphy@arm.com (Robin Murphy)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] iommu/dma: Map scatterlists more parsimoniously
Date: Wed, 11 Nov 2015 14:54:16 +0000	[thread overview]
Message-ID: <104b4682056bc5b4aa0afadd39196c03fc06555d.1447249098.git.robin.murphy@arm.com> (raw)

Whilst blindly assuming the worst case for segment boundaries and
aligning every segment individually is safe from the point of view
of respecting the device's parameters, it is also undeniably a waste
of IOVA space. Futhermore, the knock-on effects of more pages than
necessary being exposed to device access, additional overhead in page
table updates and TLB invalidations, etc., are even more undesirable.

Improve matters by taking the actual boundary mask into account to
actively detect the cases in which we really do need to adjust a
segment, and avoid wasting space in the remainder.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---

Hi all,

I've given this some brief testing on Juno with USB and (via magic
PCI hacks) SATA to confirm that all the +1s and -1s at least seem to
be in the right places, so I'm throwing it out now for a head-start on
checking whether it also helps the media folks with the v4l portability
issues they're up against (I'm confident it should). If all goes well I
figure I'll repost next week based on rc1 instead of some random local
development commit.

Robin.

 drivers/iommu/dma-iommu.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 3a20db4..821ebc4 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -441,6 +441,7 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	struct scatterlist *s, *prev = NULL;
 	dma_addr_t dma_addr;
 	size_t iova_len = 0;
+	unsigned long mask = dma_get_seg_boundary(dev);
 	int i;
 
 	/*
@@ -460,17 +461,19 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
 		s->length = s_length;
 
 		/*
-		 * The simple way to avoid the rare case of a segment
-		 * crossing the boundary mask is to pad the previous one
-		 * to end at a naturally-aligned IOVA for this one's size,
-		 * at the cost of potentially over-allocating a little.
+		 * With a single size-aligned IOVA allocation, no segment risks
+		 * crossing the boundary mask unless the total size exceeds
+		 * the mask itself. The simple way to maintain alignment when
+		 * that does happen is to pad the previous segment to end at the
+		 * next boundary, at the cost of over-allocating a little.
 		 */
 		if (prev) {
-			size_t pad_len = roundup_pow_of_two(s_length);
+			size_t pad_len = (mask - iova_len + 1) & mask;
 
-			pad_len = (pad_len - iova_len) & (pad_len - 1);
-			prev->length += pad_len;
-			iova_len += pad_len;
+			if (pad_len && pad_len < s_length - 1) {
+				prev->length += pad_len;
+				iova_len += pad_len;
+			}
 		}
 
 		iova_len += s_length;
-- 
1.9.1

             reply	other threads:[~2015-11-11 14:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-11 14:54 Robin Murphy [this message]
2015-11-11 14:54 ` [PATCH] iommu/dma: Map scatterlists more parsimoniously Robin Murphy
     [not found] ` <104b4682056bc5b4aa0afadd39196c03fc06555d.1447249098.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2015-11-12  5:47   ` Yong Wu
2015-11-12  5:47     ` Yong Wu
2015-12-10 12:49     ` Daniel Kurtz
2015-12-10 12:49       ` Daniel Kurtz
     [not found]       ` <CAGS+omBWFgpQQKV2=qL9G6qPbW_EL2bSjkLPq6KwQGAdHEDSHw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-02  7:04         ` Daniel Kurtz
2016-02-02  7:04           ` Daniel Kurtz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=104b4682056bc5b4aa0afadd39196c03fc06555d.1447249098.git.robin.murphy@arm.com \
    --to=robin.murphy-5wv7dgnigg8@public.gmane.org \
    --cc=djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=tiffany.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.