Message-Id: <200212311500.gBVF0qZ01875@localhost.localdomain>
To: David Brownell <david-b@pacbell.net>
cc: James Bottomley <James.Bottomley@SteelEye.com>,
       linux-kernel@vger.kernel.org
Subject: Re: [PATCH] generic device DMA (dma_pool update) 
In-Reply-To: Message from David Brownell <david-b@pacbell.net> 
   of "Mon, 30 Dec 2002 15:11:19 PST." <3E10D297.1090206@pacbell.net> 
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Date: Tue, 31 Dec 2002 09:00:50 -0600
From: James Bottomley <James.Bottomley@steeleye.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1868
Lines: 54

I think a much easier way of doing this would be a slight modification to the 
current pci_pool code: we know it works already.  All that really has to 
change is that it should take a struct device instead of a pci_dev and it 
should call dma_alloc_coherent to get the source memory.  The rest of the pci 
pool code is mainly about memory management and is well tested and so should 
remain (much as I'd like to see a mempool implementation).

You have this in your code:

+void *
+dma_alloc (struct device *dev, size_t size, dma_addr_t *handle, int mem_flags)
+{
+	struct pci_dev	*pdev = to_pci_dev(dev);
+
+	BUG_ON(dev->bus != &pci_bus_type);
+
+	if (size >= PAGE_SIZE)
+		return pci_alloc_consistent (pdev, size, handle);

This would work if just passed to dma_alloc_coherent and drop the check for 
the pci_bus_type (If the dma_ APIs are implemented in terms of the pci_ ones, 
then they will bug out in the generic implementation of dma_alloc_coherent so 
there's no need to do this type of transform twice).

This:

+dma_alloc (struct device *dev, size_t size, dma_addr_t *handle, int mem_flags)
+{
+	void	*ret;
+
+	/* We rely on kmalloc memory being aligned to L1_CACHE_BYTES, to
+	 * prevent cacheline sharing during DMA.  With dma-incoherent caches,
+	 * such sharing causes bugs, not just cache-related slowdown.
+	 */
+	ret = kmalloc (size, mem_flags | __dma_slab_flags (dev));
+	if (likely (ret != 0))
+		*handle = virt_to_phys (ret);
+	return ret;

Just can't be done: you can't get consistent memory from kmalloc and you 
certainly can't use virt_to_phys and expect it to work on IOMMU hardware.

James


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/