Re: [Xen-devel] [PATCH v2 2/2] xen/m2p: use GNTTABOP_unmap_and_replace to reinstate the original mapping

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Mon, Jul 22, 2013 at 06:02:45PM +0100, David Vrabel wrote:
> On 22/07/13 17:28, Stefano Stabellini wrote:
> > GNTTABOP_unmap_grant_ref unmaps a grant and replaces it with a 0
> > mapping instead of reinstating the original mapping.
> > Doing so separately would be racy.
> > 
> > To unmap a grant and reinstate the original mapping atomically we use
> > GNTTABOP_unmap_and_replace.
> > GNTTABOP_unmap_and_replace doesn't work with GNTMAP_contains_pte, so
> > don't use it for kmaps.  GNTTABOP_unmap_and_replace zeroes the mapping
> > passed in new_addr so we have to reinstate it, however that is a
> > per-cpu mapping only used for balloon trade pages, so we can be sure that
> > it's not going to be accessed while the mapping is not valid.
> 
> This solves the problem of userspace accessing a disk image on an NFS
> mount but what would blkback talking to an iSCSI LUN?  Will that need
> similar fixes to blkback?  This series does not need to fix this now though.

I am not sure I follow. The blkback using iSCSI LUNs works just fine
(I am using that - I have LVs of guests on an iSCSI disk).

I think the problem you are alluding to is when blkfront and netback are both
involved. Meaning you have an iSCSI target in an PV DomU guest exporting
its LUNs to other domU guests. The iSCSI target resides on top of blkfront.

That means that within the guest you have - netback and blkfront using
the same M2P entry. That is a seperate issue I think that this patch.

But I could also be misremembering the problem statement. Roger had
encountered it and dig some diagnosis of it.

> 
> > --- a/arch/x86/xen/p2m.c
> > +++ b/arch/x86/xen/p2m.c
> > @@ -161,6 +161,7 @@
> >  #include <asm/xen/page.h>
> >  #include <asm/xen/hypercall.h>
> >  #include <asm/xen/hypervisor.h>
> > +#include <xen/balloon.h>
> >  #include <xen/grant_table.h>
> >  
> >  #include "multicalls.h"
> > @@ -967,7 +968,9 @@ int m2p_remove_override(struct page *page,
> >     if (kmap_op != NULL) {
> >             if (!PageHighMem(page)) {
> >                     struct multicall_space mcs;
> > -                   struct gnttab_unmap_grant_ref *unmap_op;
> > +                   struct gnttab_unmap_and_replace *unmap_op;
> > +                   unsigned long trade_page_address = (unsigned long)
> > +                           __va(page_to_pfn(get_balloon_trade_page()) << 
> > PAGE_SHIFT);
> 
>       struct page *trade_page = get_balloon_trade_page();
>       unsigned long trade_page_address = page_address(trade_page);
> 
> (And the corresponding put_balloon_trade_page() once you've added it.)
> 
> Otherwise,
> 
> Reviewed-by: David Vrabel <david.vrabel@xxxxxxxxxx>
> 
> David

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel