Re: [Xen-devel] [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> Sent: 27 November 2018 15:48
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Xen-devel <xen-
> devel@xxxxxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Ian Jackson
> <Ian.Jackson@xxxxxxxxxx>
> Subject: Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> idempotent
> 
> On Sat, Nov 24, 2018 at 03:09:33PM +0000, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf
> > > Of Paul Durrant
> > > Sent: 24 November 2018 15:06
> > > To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Xen-devel <xen-
> > > devel@xxxxxxxxxxxxx>
> > > Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu
> > > <wei.liu2@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>
> > > Subject: Re: [Xen-devel] [PATCH] tools/libs: Make
> > > xenforeignmemory_unmap_resource() idempotent
> > >
> > > > -----Original Message-----
> > > > From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> > > > Sent: 23 November 2018 15:12
> > > > To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> > > > Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson
> > > > <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Paul
> Durrant
> > > > <Paul.Durrant@xxxxxxxxxx>
> > > > Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> > > > idempotent
> > > >
> > > > Most other close/unmap functions are.
> > > >
> > > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > > ---
> > > > CC: Ian Jackson <Ian.Jackson@xxxxxxxxxx>
> > > > CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > > >
> > > > This ideally wants backporting to 4.11 to hit 4.11.1
> > > >
> > > > I got an unexpected shock while trying to diagnose why GVT-g is
> still
> > > > broken (differently!) on staging.
> > > > ---
> > > >  tools/libs/foreignmemory/core.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/libs/foreignmemory/core.c
> > > > b/tools/libs/foreignmemory/core.c
> > > > index 63f12e2..2516fd4 100644
> > > > --- a/tools/libs/foreignmemory/core.c
> > > > +++ b/tools/libs/foreignmemory/core.c
> > > > @@ -182,7 +182,12 @@ xenforeignmemory_resource_handle
> > > > *xenforeignmemory_map_resource(
> > > >  int xenforeignmemory_unmap_resource(
> > > >      xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle
> > > > *fres)
> > > >  {
> > > > -    int rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> > > > +    int rc;
> > > > +
> > > > +    if ( !fres )
> > > > +        return 0;
> > > > +
> > > > +    rc = osdep_xenforeignmemory_unmap_resource(fmem, fres);
> > >
> > > Freeing NULL should not be problem as this defined to do nothing so
> there
> > > is nothing about this function which is not idempotent. I assume,
> without
> > > looking yet, that it is the osdep function which needs fixing.
> > >
> >
> > ...and indeed it does. I think this is patch you need...
> >
> > diff --git a/tools/libs/foreignmemory/linux.c
> b/tools/libs/foreignmemory/linux.c
> > index 132875d..8daa582 100644
> > --- a/tools/libs/foreignmemory/linux.c
> > +++ b/tools/libs/foreignmemory/linux.c
> > @@ -298,7 +298,7 @@ int
> osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
> >  int osdep_xenforeignmemory_unmap_resource(
> >      xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle
> *fres)
> >  {
> > -    return munmap(fres->addr, fres->nr_frames << PAGE_SHIFT);
> > +    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) :
> 0;
> 
> Can you submit a patch for this? And please do this for other osdep
> functions as well.
> 

Sure.

  Paul

> Wei.
> 
> >  }
> >
> >   Paul

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel