Re: Re: blktap: Sync with XCP, dropping zero-copy.

[Daniel Stodden <daniel.stodden@citrix.com>]

On Wed, 2010-11-17 at 12:04 -0500, Ian Campbell wrote:
> On Tue, 2010-11-16 at 21:28 +0000, Daniel Stodden wrote:
> > 
> > > > Also, I was absolutely certain I once saw VM_FOREIGN support in
> > gntdev..
> > > > Can't find it now, what happened? Without, there's presently still
> > no
> > > > zero-copy.
> > > 
> > > gntdev doesn't need VM_FOREIGN any more - it uses the (relatively
> > > new-ish) mmu notifier infrastructure which is intended to allow a
> > device
> > > to sync an external MMU with usermode mappings.  We're not using it
> > in
> > > precisely that way, but it allows us to wrangle grant mappings
> > before
> > > the generic code tries to do normal pte ops on them.
> > 
> > The mmu notifiers were for safe teardown only. They are not sufficient
> > for DIO, which wants gup() to work. If you want zcopy on gntdev, we'll
> > need to back those VMAs with page structs.  Or bounce again (gulp,
> > just mentioning it). As with the blktap2 patches, note there is no
> > difference in the dom0 memory bill, it takes page frames.
> 
> I though the VM_FOREIGN stuff which blktap[12] hooks into gup() was
> there in order to avoid some deadlock arising from having a single
> struct page taking part in nested block I/O. i.e. one I/O created by
> blkback and the second from the tapdisk process deadlock against each
> other.



> I though that in order to work around this blktap creates a second range
> of struct pages (the struct vm_foreign_map array in
> vma->vm_private_data) which aliases the pages under I/O and then it
> hooks gup() to do the switcheroo when necessary.
> 
> If the blkback interface is running in userspace (whether in a tapdisk
> or qemu process) then there is no nesting of I/O and the only I/O is
> from process context and therefore this particular issue is no longer a
> problem because we can use a properly struct page backed page without
> needing a magic VM_FOREIGN shadow of it?
> 
> Have I misunderstood something about the reason for VM_FOREIGN?

VM_FOREIGN is a special case of grant mapping frames into a user's VMA.

This stuff is primarily there to make gup() grab a pagevec from the VMA
struct, instead of relying on follow_page in order to map user-virtual
to pseudophysical, which is what gup normally does.

In brief, it's hacking gup() to keep DIO working. 

In other words: Userland can't I/O on some VM_PFNMAP or VM_IO or
similar. If you ask for DMA, the kernel quite urgently wants this to
look like normal memory.

Your description of the aliasing is correct, but that's yet another
thing. It implies redoing the grantmap and p2m entries privately.

To make this clearer: If the aliasing weren't necessary, blktap2 would
just have had to grab blkback's prepared page struct from the request SG
vector, and .mmap that to userland, but still with VM_FOREIGN and some
pagevec pointer in the VMA.

Instead, there's blkback-pagemap, specifically to map that SG page entry
*back* to the original gref from a table, and *redo* the entire gntmap +
p2m thing another time, privately.

Twisted, agreed. :)

Cheers,
Daniel