[Xen-devel] SWIOMMU redundant copies?

["Stephen C. Tweedie" <sct@xxxxxxxxxx>]

Hi,

In current linux-2.6.18-xen.h 64-bit, it looks like we're doing
potentially large amounts of unnecessary IO bounce-buffering.

I noticed this when trying to pin down some odd behaviour concerning
changeset 15405 in xen-3.1-testing:

        http://xenbits.xensource.com/xen-3.1-testing.hg?rev/30033a637942

        # User kfraser@xxxxxxxxxxxxxxxxxxxxx
        # Date 1183985110 -3600
        # Node ID 30033a6379428f57269455f0963841743c6d5e46
        # Parent  9cdade953890a612734d97b3abc21513a1a9cf6d
        x86: dma_map_sg() must handle multi-page segments.
        Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx>

which adds a test when we do swiotlb_map_sg():

--- a/linux-2.6-xen-sparse/arch/i386/kernel/swiotlb.c   Fri Sep 14 16:33:44 
2007 +0100
+++ b/linux-2.6-xen-sparse/arch/i386/kernel/swiotlb.c   Mon Jul 09 13:45:10 
2007 +0100
@@ -572,7 +572,9 @@ swiotlb_map_sg(struct device *hwdev, str
 
        for (i = 0; i < nelems; i++, sg++) {
                dev_addr = SG_ENT_PHYS_ADDRESS(sg);
-               if (address_needs_mapping(hwdev, dev_addr)) {
+               if (range_straddles_page_boundary(page_to_pseudophys(sg->page)
+                                                 + sg->offset, sg->length)
+                   || address_needs_mapping(hwdev, dev_addr)) {
                        buffer.page   = sg->page;

This is still present in linux-2.6.18-xen.hg's lib/swiotlb-xen.c; but it
seems to conflict with a different test that we use when we merge bios
together for multi-page disk IO, where we have

include/asm-x86_64/mach-xen/asm/io.h:
#define page_to_pseudophys(page) ((dma_addr_t)page_to_pfn(page) << PAGE_SHIFT)
#define page_to_phys(page)       (phys_to_machine(page_to_pseudophys(page)))
#define page_to_bus(page)        (phys_to_machine(page_to_pseudophys(page)))

#define bio_to_pseudophys(bio)   (page_to_pseudophys(bio_page((bio))) + \
                                  (unsigned long) bio_offset((bio)))
#define bvec_to_pseudophys(bv)   (page_to_pseudophys((bv)->bv_page) + \
                                  (unsigned long) (bv)->bv_offset)

#define BIOVEC_PHYS_MERGEABLE(vec1, vec2)       \
        (((bvec_to_phys((vec1)) + (vec1)->bv_len) == bvec_to_phys((vec2))) && \
         ((bvec_to_pseudophys((vec1)) + (vec1)->bv_len) == \
          bvec_to_pseudophys((vec2))))

This test in io.h basically means that bios can, and will, span page
boundaries if the pages in question are both physical- and machine-
contiguous; but the range_straddles_page_boundary test does NOT check
for machine-contiguous pages (it only tests whether pages are in a
xen_create_contiguous_region() region, not whether pages happen to be
contiguous by chance in non-guaranteed-contig regions.)

So we'll create bios which span multiple pages, assuming that that's
more efficient; but then we'll force them to be copied because they span
multiple pages!

Have I missed something, or is this an oversight in the swiotlb.c patch
above?

The reason I discovered this is that including the patch seems to be
causing SWIOTLB overflows on some hardware.  Adding an extra test to
swiotlb_map_sg() to catch sg segments which are truly machine-contig and
don't need mapping, and avoid copying those, cures those symptoms for
me, but I suspect there may be other issues lurking in this area.

Cheers,
 Stephen



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel