All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent
@ 2018-11-23 15:12 Andrew Cooper
  2018-11-24 15:05 ` Paul Durrant
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2018-11-23 15:12 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Paul Durrant, Wei Liu, Ian Jackson

Most other close/unmap functions are.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Paul Durrant <paul.durrant@citrix.com>

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);
 
     free(fres);
     return rc;
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent
  2018-11-23 15:12 [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent Andrew Cooper
@ 2018-11-24 15:05 ` Paul Durrant
  2018-11-24 15:09   ` Paul Durrant
  2018-11-27 16:10   ` Andrew Cooper
  0 siblings, 2 replies; 7+ messages in thread
From: Paul Durrant @ 2018-11-24 15:05 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Ian Jackson

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 23 November 2018 15:12
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Paul Durrant
> <Paul.Durrant@citrix.com>
> Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> idempotent
> 
> Most other close/unmap functions are.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Jackson <Ian.Jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> 
> 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.

  Paul

> 
>      free(fres);
>      return rc;
> --
> 2.1.4


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent
  2018-11-24 15:05 ` Paul Durrant
@ 2018-11-24 15:09   ` Paul Durrant
  2018-11-27 15:48     ` Wei Liu
  2018-11-27 16:10   ` Andrew Cooper
  1 sibling, 1 reply; 7+ messages in thread
From: Paul Durrant @ 2018-11-24 15:09 UTC (permalink / raw)
  To: Paul Durrant, Xen-devel; +Cc: Andrew Cooper, Wei Liu, Ian Jackson

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Paul Durrant
> Sent: 24 November 2018 15:06
> To: Andrew Cooper <Andrew.Cooper3@citrix.com>; Xen-devel <xen-
> devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu
> <wei.liu2@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>
> Subject: Re: [Xen-devel] [PATCH] tools/libs: Make
> xenforeignmemory_unmap_resource() idempotent
> 
> > -----Original Message-----
> > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> > Sent: 23 November 2018 15:12
> > To: Xen-devel <xen-devel@lists.xen.org>
> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> > <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Paul Durrant
> > <Paul.Durrant@citrix.com>
> > Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> > idempotent
> >
> > Most other close/unmap functions are.
> >
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > CC: Ian Jackson <Ian.Jackson@citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > CC: Paul Durrant <paul.durrant@citrix.com>
> >
> > 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;
 }

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent
  2018-11-24 15:09   ` Paul Durrant
@ 2018-11-27 15:48     ` Wei Liu
  2018-11-27 15:50       ` Paul Durrant
  0 siblings, 1 reply; 7+ messages in thread
From: Wei Liu @ 2018-11-27 15:48 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Xen-devel

On Sat, Nov 24, 2018 at 03:09:33PM +0000, Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> > Of Paul Durrant
> > Sent: 24 November 2018 15:06
> > To: Andrew Cooper <Andrew.Cooper3@citrix.com>; Xen-devel <xen-
> > devel@lists.xen.org>
> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu
> > <wei.liu2@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>
> > Subject: Re: [Xen-devel] [PATCH] tools/libs: Make
> > xenforeignmemory_unmap_resource() idempotent
> > 
> > > -----Original Message-----
> > > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> > > Sent: 23 November 2018 15:12
> > > To: Xen-devel <xen-devel@lists.xen.org>
> > > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> > > <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Paul Durrant
> > > <Paul.Durrant@citrix.com>
> > > Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> > > idempotent
> > >
> > > Most other close/unmap functions are.
> > >
> > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > > ---
> > > CC: Ian Jackson <Ian.Jackson@citrix.com>
> > > CC: Wei Liu <wei.liu2@citrix.com>
> > > CC: Paul Durrant <paul.durrant@citrix.com>
> > >
> > > 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.

Wei.

>  }
> 
>   Paul

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent
  2018-11-27 15:48     ` Wei Liu
@ 2018-11-27 15:50       ` Paul Durrant
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Durrant @ 2018-11-27 15:50 UTC (permalink / raw)
  Cc: Andrew Cooper, Ian Jackson, Wei Liu, Xen-devel

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 27 November 2018 15:48
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Xen-devel <xen-
> devel@lists.xen.org>; Wei Liu <wei.liu2@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>
> 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@lists.xenproject.org] On
> Behalf
> > > Of Paul Durrant
> > > Sent: 24 November 2018 15:06
> > > To: Andrew Cooper <Andrew.Cooper3@citrix.com>; Xen-devel <xen-
> > > devel@lists.xen.org>
> > > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu
> > > <wei.liu2@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>
> > > Subject: Re: [Xen-devel] [PATCH] tools/libs: Make
> > > xenforeignmemory_unmap_resource() idempotent
> > >
> > > > -----Original Message-----
> > > > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> > > > Sent: 23 November 2018 15:12
> > > > To: Xen-devel <xen-devel@lists.xen.org>
> > > > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> > > > <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Paul
> Durrant
> > > > <Paul.Durrant@citrix.com>
> > > > Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> > > > idempotent
> > > >
> > > > Most other close/unmap functions are.
> > > >
> > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > > > ---
> > > > CC: Ian Jackson <Ian.Jackson@citrix.com>
> > > > CC: Wei Liu <wei.liu2@citrix.com>
> > > > CC: Paul Durrant <paul.durrant@citrix.com>
> > > >
> > > > 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent
  2018-11-24 15:05 ` Paul Durrant
  2018-11-24 15:09   ` Paul Durrant
@ 2018-11-27 16:10   ` Andrew Cooper
  2018-11-27 16:26     ` Paul Durrant
  1 sibling, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2018-11-27 16:10 UTC (permalink / raw)
  To: Paul Durrant, Xen-devel; +Cc: Ian Jackson, Wei Liu

On 24/11/2018 15:05, Paul Durrant wrote:
>> -----Original Message-----
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: 23 November 2018 15:12
>> To: Xen-devel <xen-devel@lists.xen.org>
>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
>> <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Paul Durrant
>> <Paul.Durrant@citrix.com>
>> Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
>> idempotent
>>
>> Most other close/unmap functions are.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Ian Jackson <Ian.Jackson@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Paul Durrant <paul.durrant@citrix.com>
>>
>> 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.

Why?  You can fix it once for the entire library here, or once in every
osdep flavour.

One of these is rather less code...

~Andrew

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent
  2018-11-27 16:10   ` Andrew Cooper
@ 2018-11-27 16:26     ` Paul Durrant
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Durrant @ 2018-11-27 16:26 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: Ian Jackson, Wei Liu

> -----Original Message-----
> From: Andrew Cooper
> Sent: 27 November 2018 16:10
> To: Paul Durrant <Paul.Durrant@citrix.com>; Xen-devel <xen-
> devel@lists.xen.org>
> Cc: Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> idempotent
> 
> On 24/11/2018 15:05, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> >> Sent: 23 November 2018 15:12
> >> To: Xen-devel <xen-devel@lists.xen.org>
> >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> >> <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Paul Durrant
> >> <Paul.Durrant@citrix.com>
> >> Subject: [PATCH] tools/libs: Make xenforeignmemory_unmap_resource()
> >> idempotent
> >>
> >> Most other close/unmap functions are.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Ian Jackson <Ian.Jackson@citrix.com>
> >> CC: Wei Liu <wei.liu2@citrix.com>
> >> CC: Paul Durrant <paul.durrant@citrix.com>
> >>
> >> 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.
> 
> Why?  You can fix it once for the entire library here, or once in every
> osdep flavour.
> 
> One of these is rather less code...

Actually I can argue my patch is both complete and smaller since there is a single non-idempotent osdep implementation at the moment :-) I'd also prefer that the osdep implementations are made (and kept) idempotent anyway.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-11-27 16:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-23 15:12 [PATCH] tools/libs: Make xenforeignmemory_unmap_resource() idempotent Andrew Cooper
2018-11-24 15:05 ` Paul Durrant
2018-11-24 15:09   ` Paul Durrant
2018-11-27 15:48     ` Wei Liu
2018-11-27 15:50       ` Paul Durrant
2018-11-27 16:10   ` Andrew Cooper
2018-11-27 16:26     ` Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.