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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 27 November 2018 16:10
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Xen-devel <xen-
> devel@xxxxxxxxxxxxx>
> Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>
> 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@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.
> 
> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel