Re: fuse, get_user_pages, flush_anon_page, aliasing caches and all that again

[Dan Williams]

On 1/1/07, Russell King <rmk+lkml@xxxxxxxxxxxxxxxx> wrote:
> On Mon, Jan 01, 2007 at 11:15:04PM +0100, Miklos Szeredi wrote:
> > > > > I'm willing to do that - and I guess this means we can probably do this
> > > > > instead of walking the list of VMAs for the shared mapping, thereby
> > > > > hitting both anonymous and shared mappings with the same code?
> > > >
> > > > But for the get_user_pages() case there's no point, is there?  The VMA
> > > > and the virtual address is already available, so trying to find it
> > > > again through RMAP doesn't much make sense.
> > > >
> > > > Users of get_user_pages() don't care about any other mappings (maybe
> > > > ptrace does, I don't know) only about one single user mapping and one
> > > > kernel mapping.
> > > >
> > > > So using flush_dcache_page() there is an overkill, trying to teach it
> > > > about anonymous pages is not the real solution, flush_dcache_page()
> > > > was never meant to be used on anything but file mapped pages.
> > >
> > > It's not actually.  For flush_anon_page() we currently have to flush the
> > > user mapping and the kernel mapping.  For flush_dcache_page(), it's
> > > exactly the same - we have to flush the kernel mapping and the user
> > > mapping.
> >
> > I was never advocating flush_anon_page().  I was suggesting a _new_
> > cache operation:
> >
> >    flush_kernel_user_page(page, vma, virt_addr)
> >
> > which flushes the kernel mapping and the given user mapping.  Just
> > like flush_dcache_page() but without needing to find the user
> > mapping(s).
>
> There's a problem with defining cache coherency macros to that extent.
> You take away flexibility to efficiently implement them on various
> platforms which you didn't think about (eg, in the above case it's
> perfectly fine for VIVT, but not really VIPT.)
>
> > However the cache flushing in kmap/kunmap idea might be cleaner and
> > better.
>
> It has the significant advantage that, unlike the flush* calls, they
> can't really be forgotten by folk programming on cache alias-free
> hardware.  That's a _very_ persuasive argument for this proposed
> interface.
If the interface is going to change it seems like an opportunity to
add some helper routines for the dma to user case?  Russell
illuminated some of the mines on the field in this thread:
http://marc.theaimsgroup.com/?l=linux-arm-kernel&m=116669504732601&w=2,
but I think the concerns can be addressed.

I think it is important to note that I am talking specifically about
the offloaded memcpy (i.e. NET_DMA) case and not the capability to
mmap a general purpose dma region.  Since the dma transfer is kernel
initiated (versus device initiated) the modified dma api could
properly manage cache coherency.

I was at a loss at how to handle the MAP_SHARED case, but if those
mappings are non-virtually cacheable by default, as DavidM suggested,
then that particular problem goes away.

Now it sounds like the data corruption problem I ran into on ARM will
go away with the anonymous memory handling changes to the
get_user_pages path.  However, get_user_pages is overkill for the dma
memcpy case since it can optimize for transfer size and direction.

A rough sketch of the interface would be something like the following
struct dma_addr_user {
  dma_addr_t dma;
  void *user;
};
struct dma_addr_user *dma_map_single_user(struct device *dev, void
*kernel_addr, size_t size, enum dma_data_direction dir, void
*user_addr)

Comments?
-
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/