Re: pci_map_sg() does not coalesce adjacent physical memory? x86

[FUJITA Tomonori]

On Mon, 17 Nov 2008 19:15:32 -0800
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:

> (cc's added)
> 
> On Mon, 17 Nov 2008 22:54:33 +0100 "Leon Woestenberg" <leon.woestenberg@xxxxxxxxx> wrote:
> 
> > Hello,
> > 
> > pci_map_sg() does not coalesce the scattergather list for me on x86.
> 
> In which kernel version(s)?
> 
> > Is this expected? Documentation mentions that coalescing is typically
> > done by pci_map_sg().

Hm, what document did you read? We might need to fix it.

pci_map_sg() is not a typical place to coalesce the entries of the sg
list are physically adjacent. The block layer is the typical place.

The dma operations are free to coalesce the entries that physically
and virtually adjacent but there are not many that does.

For example, by default, on x86, only AMD GART (x86_64) dma operation
coalesces such entries.


> > Manually traversing scatterlists that describe large user space
> > allocations I found that on my system 25% reduction of list length can
> > be achieved.
> > 
> > 
> > static int sgm_map_to_table(struct sg_mapping_t *sgm)
> > {
> >         int i, j = 0;
> >         dma_addr_t addr = sg_dma_address(&sgm->sgl[0]);
> >         unsigned int len = sg_dma_len(&sgm->sgl[0]);
> >         dma_addr_t cont_addr = addr;
> >         unsigned int cont_len = len;
> >         for (i = 0; i < sgm->mapped_pages - 1; i++) {
> >                 dma_addr_t next = sg_dma_address(&sgm->sgl[i + 1]);
> >                 len = sg_dma_len(&sgm->sgl[i]);
> >                 printk(KERN_DEBUG "%04d: addr=0x%08x length=0x%08x\n",
> > i, addr, len);
> >                 /* page i + 1 is non-contiguous with page i? */
> >                 if (next != addr + len) {
> >                         /* TODO create entry here (we could overwrite i) */
> >                         printk(KERN_DEBUG "%4d: cont_addr=0x%08x
> > cont_len=0x%08x\n",
> >                                 j++, cont_addr, cont_len);
> >                         cont_addr = next;
> >                         cont_len = 0;
> >                 }
> >                 /* add page i + 1 to current contiguous block */
> >                 cont_len += len;
> >                 /* goto page i + 1 */
> >                 addr = next;
> >         }
> >         /* TODO create entry here  (we could overwrite i) */
> >         printk(KERN_DEBUG "%04d: addr=0x%08x length=0x%08x\n", i, addr, len);
> >         printk(KERN_DEBUG "%4d: cont_addr=0x%08x length=0x%08x\n",
> > j++, cont_addr, cont_len);
> > }

What's kinda of your driver? If it's a SCSI (or block) driver, you
don't need this trick.

I don't think that inventing a homegrown function to coalesce sg
entries in a driver is a good idea. If you really need this, it's
would be better to have a generic function to coalesce sg entries and
modify the block layer to use it (and your driver can use it too).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/