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);
+
+ 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.2On 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)
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
>
--
Peter Xu
© 2016 - 2026 Red Hat, Inc.