| Message ID | 20241022200830.4129598-2-alex.williamson@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vfio: Align mmaps | expand |
On Tue, Oct 22, 2024 at 02:08:28PM -0600, Alex Williamson wrote: > Move error handling code to the end of the function so that it can more > easily be shared by new mmap failure conditions. No functional change > intended. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > hw/vfio/helpers.c | 34 +++++++++++++++++----------------- > 1 file changed, 17 insertions(+), 17 deletions(-) > > diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c > index ea15c79db0a3..b9e606e364a2 100644 > --- a/hw/vfio/helpers.c > +++ b/hw/vfio/helpers.c > @@ -395,7 +395,7 @@ static void vfio_subregion_unmap(VFIORegion *region, int index) > > int vfio_region_mmap(VFIORegion *region) > { > - int i, prot = 0; > + int i, ret, prot = 0; > char *name; > > if (!region->mem) { > @@ -411,22 +411,8 @@ int vfio_region_mmap(VFIORegion *region) > region->fd_offset + > region->mmaps[i].offset); > if (region->mmaps[i].mmap == MAP_FAILED) { > - int ret = -errno; > - > - trace_vfio_region_mmap_fault(memory_region_name(region->mem), i, > - region->fd_offset + > - region->mmaps[i].offset, > - region->fd_offset + > - region->mmaps[i].offset + > - region->mmaps[i].size - 1, ret); > - > - region->mmaps[i].mmap = NULL; > - > - for (i--; i >= 0; i--) { > - vfio_subregion_unmap(region, i); > - } > - > - return ret; > + ret = -errno; > + goto no_mmap; > } > > name = g_strdup_printf("%s mmaps[%d]", > @@ -446,6 +432,20 @@ int vfio_region_mmap(VFIORegion *region) > } > > return 0; > + > +no_mmap: > + trace_vfio_region_mmap_fault(memory_region_name(region->mem), i, > + region->fd_offset + region->mmaps[i].offset, > + region->fd_offset + region->mmaps[i].offset + > + region->mmaps[i].size - 1, ret); While at it, maybe when moving we can rename s/fault/fail/ or some other words to avoid "mmap" + "fault" being together. :) Totally not a huge deal. Reviewed-by: Peter Xu <peterx@redhat.com> > + > + region->mmaps[i].mmap = NULL; > + > + for (i--; i >= 0; i--) { > + vfio_subregion_unmap(region, i); > + } > + > + return ret; > } > > void vfio_region_unmap(VFIORegion *region) > -- > 2.46.2 >
On 10/22/24 22:08, Alex Williamson wrote: > Move error handling code to the end of the function so that it can more > easily be shared by new mmap failure conditions. No functional change > intended. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > hw/vfio/helpers.c | 34 +++++++++++++++++----------------- > 1 file changed, 17 insertions(+), 17 deletions(-) > > diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c > index ea15c79db0a3..b9e606e364a2 100644 > --- a/hw/vfio/helpers.c > +++ b/hw/vfio/helpers.c > @@ -395,7 +395,7 @@ static void vfio_subregion_unmap(VFIORegion *region, int index) > > int vfio_region_mmap(VFIORegion *region) > { > - int i, prot = 0; > + int i, ret, prot = 0; > char *name; > > if (!region->mem) { > @@ -411,22 +411,8 @@ int vfio_region_mmap(VFIORegion *region) > region->fd_offset + > region->mmaps[i].offset); > if (region->mmaps[i].mmap == MAP_FAILED) { > - int ret = -errno; > - > - trace_vfio_region_mmap_fault(memory_region_name(region->mem), i, > - region->fd_offset + > - region->mmaps[i].offset, > - region->fd_offset + > - region->mmaps[i].offset + > - region->mmaps[i].size - 1, ret); > - > - region->mmaps[i].mmap = NULL; > - > - for (i--; i >= 0; i--) { > - vfio_subregion_unmap(region, i); > - } > - > - return ret; > + ret = -errno; > + goto no_mmap; > } > > name = g_strdup_printf("%s mmaps[%d]", > @@ -446,6 +432,20 @@ int vfio_region_mmap(VFIORegion *region) > } > > return 0; > + > +no_mmap: > + trace_vfio_region_mmap_fault(memory_region_name(region->mem), i, > + region->fd_offset + region->mmaps[i].offset, > + region->fd_offset + region->mmaps[i].offset + > + region->mmaps[i].size - 1, ret); > + > + region->mmaps[i].mmap = NULL; > + > + for (i--; i >= 0; i--) { > + vfio_subregion_unmap(region, i); > + } > + > + return ret; > } > > void vfio_region_unmap(VFIORegion *region)
diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c index ea15c79db0a3..b9e606e364a2 100644 --- a/hw/vfio/helpers.c +++ b/hw/vfio/helpers.c @@ -395,7 +395,7 @@ static void vfio_subregion_unmap(VFIORegion *region, int index) int vfio_region_mmap(VFIORegion *region) { - int i, prot = 0; + int i, ret, prot = 0; char *name; if (!region->mem) { @@ -411,22 +411,8 @@ int vfio_region_mmap(VFIORegion *region) region->fd_offset + region->mmaps[i].offset); if (region->mmaps[i].mmap == MAP_FAILED) { - int ret = -errno; - - trace_vfio_region_mmap_fault(memory_region_name(region->mem), i, - region->fd_offset + - region->mmaps[i].offset, - region->fd_offset + - region->mmaps[i].offset + - region->mmaps[i].size - 1, ret); - - region->mmaps[i].mmap = NULL; - - for (i--; i >= 0; i--) { - vfio_subregion_unmap(region, i); - } - - return ret; + ret = -errno; + goto no_mmap; } name = g_strdup_printf("%s mmaps[%d]", @@ -446,6 +432,20 @@ int vfio_region_mmap(VFIORegion *region) } return 0; + +no_mmap: + trace_vfio_region_mmap_fault(memory_region_name(region->mem), i, + region->fd_offset + region->mmaps[i].offset, + region->fd_offset + region->mmaps[i].offset + + region->mmaps[i].size - 1, ret); + + region->mmaps[i].mmap = NULL; + + for (i--; i >= 0; i--) { + vfio_subregion_unmap(region, i); + } + + return ret; } void vfio_region_unmap(VFIORegion *region)
Move error handling code to the end of the function so that it can more easily be shared by new mmap failure conditions. No functional change intended. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/vfio/helpers.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)