[Mesa-dev] [PATCH 1/2] vulkan/wsi: Store the instance allocator in wsi_device

Eric Engestrom eric.engestrom at intel.com
Thu Oct 18 10:40:11 UTC 2018


On Wednesday, 2018-10-17 15:14:03 -0500, Jason Ekstrand wrote:
> We already have wsi_device and we know the instance allocator at
> wsi_device_init time so there's no need to pass it into the physical
> device queries.  This also fixes a memory allocation domain bug that can
> occur if CreateSwapchain gets called prior to any queries (not likely)
> in which case the cached connection gets allocated off the device
> instead of the instance.
> ---
>  src/amd/vulkan/radv_wsi.c           |  1 -
>  src/amd/vulkan/radv_wsi_x11.c       |  2 --
>  src/intel/vulkan/anv_wsi.c          |  1 -
>  src/intel/vulkan/anv_wsi_x11.c      |  2 --
>  src/vulkan/wsi/wsi_common.c         |  4 ++--
>  src/vulkan/wsi/wsi_common.h         |  4 +++-
>  src/vulkan/wsi/wsi_common_display.c |  1 -
>  src/vulkan/wsi/wsi_common_private.h |  1 -
>  src/vulkan/wsi/wsi_common_wayland.c |  1 -
>  src/vulkan/wsi/wsi_common_x11.c     | 25 +++++++++++--------------
>  src/vulkan/wsi/wsi_common_x11.h     |  1 -
>  11 files changed, 16 insertions(+), 27 deletions(-)
> 
> diff --git a/src/amd/vulkan/radv_wsi.c b/src/amd/vulkan/radv_wsi.c
> index 6479bea070b..8b165ea3916 100644
> --- a/src/amd/vulkan/radv_wsi.c
> +++ b/src/amd/vulkan/radv_wsi.c
> @@ -75,7 +75,6 @@ VkResult radv_GetPhysicalDeviceSurfaceSupportKHR(
>  					      device->local_fd,
>  					      queueFamilyIndex,
>  					      surface,
> -					      &device->instance->alloc,
>  					      pSupported);
>  }
>  
> diff --git a/src/amd/vulkan/radv_wsi_x11.c b/src/amd/vulkan/radv_wsi_x11.c
> index c65ac938772..9ef02ccc435 100644
> --- a/src/amd/vulkan/radv_wsi_x11.c
> +++ b/src/amd/vulkan/radv_wsi_x11.c
> @@ -44,7 +44,6 @@ VkBool32 radv_GetPhysicalDeviceXcbPresentationSupportKHR(
>  
>     return wsi_get_physical_device_xcb_presentation_support(
>        &device->wsi_device,
> -      &device->instance->alloc,
>        queueFamilyIndex,
>        device->local_fd, true,
>        connection, visual_id);
> @@ -60,7 +59,6 @@ VkBool32 radv_GetPhysicalDeviceXlibPresentationSupportKHR(
>  
>     return wsi_get_physical_device_xcb_presentation_support(
>        &device->wsi_device,
> -      &device->instance->alloc,
>        queueFamilyIndex,
>        device->local_fd, true,
>        XGetXCBConnection(dpy), visualID);
> diff --git a/src/intel/vulkan/anv_wsi.c b/src/intel/vulkan/anv_wsi.c
> index 5ed1d711689..1c9a54804e8 100644
> --- a/src/intel/vulkan/anv_wsi.c
> +++ b/src/intel/vulkan/anv_wsi.c
> @@ -92,7 +92,6 @@ VkResult anv_GetPhysicalDeviceSurfaceSupportKHR(
>                                           device->local_fd,
>                                           queueFamilyIndex,
>                                           surface,
> -                                         &device->instance->alloc,
>                                           pSupported);
>  }
>  
> diff --git a/src/intel/vulkan/anv_wsi_x11.c b/src/intel/vulkan/anv_wsi_x11.c
> index 2feb5f13376..45c43f6f17f 100644
> --- a/src/intel/vulkan/anv_wsi_x11.c
> +++ b/src/intel/vulkan/anv_wsi_x11.c
> @@ -40,7 +40,6 @@ VkBool32 anv_GetPhysicalDeviceXcbPresentationSupportKHR(
>  
>     return wsi_get_physical_device_xcb_presentation_support(
>        &device->wsi_device,
> -      &device->instance->alloc,
>        queueFamilyIndex,
>        device->local_fd, false,
>        connection, visual_id);
> @@ -56,7 +55,6 @@ VkBool32 anv_GetPhysicalDeviceXlibPresentationSupportKHR(
>  
>     return wsi_get_physical_device_xcb_presentation_support(
>        &device->wsi_device,
> -      &device->instance->alloc,
>        queueFamilyIndex,
>        device->local_fd, false,
>        XGetXCBConnection(dpy), visualID);
> diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
> index 3416fef3076..1e3c4e0028b 100644
> --- a/src/vulkan/wsi/wsi_common.c
> +++ b/src/vulkan/wsi/wsi_common.c
> @@ -39,6 +39,7 @@ wsi_device_init(struct wsi_device *wsi,
>  
>     memset(wsi, 0, sizeof(*wsi));
>  
> +   wsi->instance_alloc = *alloc;
>     wsi->pdevice = pdevice;
>  
>  #define WSI_GET_CB(func) \
> @@ -677,13 +678,12 @@ wsi_common_get_surface_support(struct wsi_device *wsi_device,
>                                 int local_fd,
>                                 uint32_t queueFamilyIndex,
>                                 VkSurfaceKHR _surface,
> -                               const VkAllocationCallbacks *alloc,
>                                 VkBool32* pSupported)
>  {
>     ICD_FROM_HANDLE(VkIcdSurfaceBase, surface, _surface);
>     struct wsi_interface *iface = wsi_device->wsi[surface->platform];
>  
> -   return iface->get_support(surface, wsi_device, alloc,
> +   return iface->get_support(surface, wsi_device,
>                               queueFamilyIndex, local_fd, pSupported);
>  }
>  
> diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h
> index 14f65097bb3..424330de566 100644
> --- a/src/vulkan/wsi/wsi_common.h
> +++ b/src/vulkan/wsi/wsi_common.h
> @@ -90,6 +90,9 @@ struct wsi_interface;
>  #define VK_ICD_WSI_PLATFORM_MAX (VK_ICD_WSI_PLATFORM_DISPLAY + 1)
>  
>  struct wsi_device {
> +   /* Allocator for the instance */
> +   VkAllocationCallbacks instance_alloc;
> +
>     VkPhysicalDevice pdevice;
>     VkPhysicalDeviceMemoryProperties memory_props;
>     uint32_t queue_family_count;
> @@ -166,7 +169,6 @@ wsi_common_get_surface_support(struct wsi_device *wsi_device,
>                                 int local_fd,
>                                 uint32_t queueFamilyIndex,
>                                 VkSurfaceKHR surface,
> -                               const VkAllocationCallbacks *alloc,
>                                 VkBool32* pSupported);
>  
>  VkResult
> diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c
> index 5cebde5b9bc..c004060a205 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -802,7 +802,6 @@ wsi_create_display_surface(VkInstance instance,
>  static VkResult
>  wsi_display_surface_get_support(VkIcdSurfaceBase *surface,
>                                  struct wsi_device *wsi_device,
> -                                const VkAllocationCallbacks *allocator,
>                                  uint32_t queueFamilyIndex,
>                                  int local_fd,
>                                  VkBool32* pSupported)
> diff --git a/src/vulkan/wsi/wsi_common_private.h b/src/vulkan/wsi/wsi_common_private.h
> index ee7ae75b8f7..accc1170163 100644
> --- a/src/vulkan/wsi/wsi_common_private.h
> +++ b/src/vulkan/wsi/wsi_common_private.h
> @@ -100,7 +100,6 @@ wsi_destroy_image(const struct wsi_swapchain *chain,
>  struct wsi_interface {
>     VkResult (*get_support)(VkIcdSurfaceBase *surface,
>                             struct wsi_device *wsi_device,
> -                           const VkAllocationCallbacks *alloc,
>                             uint32_t queueFamilyIndex,
>                             int local_fd,
>                             VkBool32* pSupported);
> diff --git a/src/vulkan/wsi/wsi_common_wayland.c b/src/vulkan/wsi/wsi_common_wayland.c
> index 6b34e21bd98..67d5f8d4c80 100644
> --- a/src/vulkan/wsi/wsi_common_wayland.c
> +++ b/src/vulkan/wsi/wsi_common_wayland.c
> @@ -464,7 +464,6 @@ wsi_wl_get_presentation_support(struct wsi_device *wsi_device,
>  static VkResult
>  wsi_wl_surface_get_support(VkIcdSurfaceBase *surface,
>                             struct wsi_device *wsi_device,
> -                           const VkAllocationCallbacks *alloc,
>                             uint32_t queueFamilyIndex,
>                             int local_fd,
>                             VkBool32* pSupported)
> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
> index 5f99b2ee473..75aa83bb7b4 100644
> --- a/src/vulkan/wsi/wsi_common_x11.c
> +++ b/src/vulkan/wsi/wsi_common_x11.c
> @@ -125,7 +125,7 @@ wsi_x11_check_dri3_compatible(xcb_connection_t *conn, int local_fd)
>  }
>  
>  static struct wsi_x11_connection *
> -wsi_x11_connection_create(const VkAllocationCallbacks *alloc,
> +wsi_x11_connection_create(struct wsi_device *wsi_dev,

Nit: I'd keep that `const`, it has no reason not to be

>                            xcb_connection_t *conn)
>  {
>     xcb_query_extension_cookie_t dri3_cookie, pres_cookie, amd_cookie, nv_cookie;
> @@ -134,7 +134,7 @@ wsi_x11_connection_create(const VkAllocationCallbacks *alloc,
>     bool has_present_v1_2 = false;
>  
>     struct wsi_x11_connection *wsi_conn =
> -      vk_alloc(alloc, sizeof(*wsi_conn), 8,
> +      vk_alloc(&wsi_dev->instance_alloc, sizeof(*wsi_conn), 8,
>                  VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
>     if (!wsi_conn)
>        return NULL;
> @@ -163,7 +163,7 @@ wsi_x11_connection_create(const VkAllocationCallbacks *alloc,
>        free(pres_reply);
>        free(amd_reply);
>        free(nv_reply);
> -      vk_free(alloc, wsi_conn);
> +      vk_free(&wsi_dev->instance_alloc, wsi_conn);
>        return NULL;
>     }
>  
> @@ -211,10 +211,10 @@ wsi_x11_connection_create(const VkAllocationCallbacks *alloc,
>  }
>  
>  static void
> -wsi_x11_connection_destroy(const VkAllocationCallbacks *alloc,
> +wsi_x11_connection_destroy(struct wsi_device *wsi_dev,

Ditto: `const`

Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>

>                             struct wsi_x11_connection *conn)
>  {
> -   vk_free(alloc, conn);
> +   vk_free(&wsi_dev->instance_alloc, conn);
>  }
>  
>  static bool
> @@ -231,7 +231,6 @@ wsi_x11_check_for_dri3(struct wsi_x11_connection *wsi_conn)
>  
>  static struct wsi_x11_connection *
>  wsi_x11_get_connection(struct wsi_device *wsi_dev,
> -		       const VkAllocationCallbacks *alloc,
>                         xcb_connection_t *conn)
>  {
>     struct wsi_x11 *wsi =
> @@ -247,7 +246,7 @@ wsi_x11_get_connection(struct wsi_device *wsi_dev,
>        pthread_mutex_unlock(&wsi->mutex);
>  
>        struct wsi_x11_connection *wsi_conn =
> -         wsi_x11_connection_create(alloc, conn);
> +         wsi_x11_connection_create(wsi_dev, conn);
>        if (!wsi_conn)
>           return NULL;
>  
> @@ -256,7 +255,7 @@ wsi_x11_get_connection(struct wsi_device *wsi_dev,
>        entry = _mesa_hash_table_search(wsi->connections, conn);
>        if (entry) {
>           /* Oops, someone raced us to it */
> -         wsi_x11_connection_destroy(alloc, wsi_conn);
> +         wsi_x11_connection_destroy(wsi_dev, wsi_conn);
>        } else {
>           entry = _mesa_hash_table_insert(wsi->connections, conn, wsi_conn);
>        }
> @@ -382,7 +381,6 @@ visual_has_alpha(xcb_visualtype_t *visual, unsigned depth)
>  
>  VkBool32 wsi_get_physical_device_xcb_presentation_support(
>      struct wsi_device *wsi_device,
> -    VkAllocationCallbacks *alloc,
>      uint32_t                                    queueFamilyIndex,
>      int fd,
>      bool can_handle_different_gpu,
> @@ -390,7 +388,7 @@ VkBool32 wsi_get_physical_device_xcb_presentation_support(
>      xcb_visualid_t                              visual_id)
>  {
>     struct wsi_x11_connection *wsi_conn =
> -      wsi_x11_get_connection(wsi_device, alloc, connection);
> +      wsi_x11_get_connection(wsi_device, connection);
>  
>     if (!wsi_conn)
>        return false;
> @@ -433,7 +431,6 @@ x11_surface_get_window(VkIcdSurfaceBase *icd_surface)
>  static VkResult
>  x11_surface_get_support(VkIcdSurfaceBase *icd_surface,
>                          struct wsi_device *wsi_device,
> -                        const VkAllocationCallbacks *alloc,
>                          uint32_t queueFamilyIndex,
>                          int local_fd,
>                          VkBool32* pSupported)
> @@ -442,7 +439,7 @@ x11_surface_get_support(VkIcdSurfaceBase *icd_surface,
>     xcb_window_t window = x11_surface_get_window(icd_surface);
>  
>     struct wsi_x11_connection *wsi_conn =
> -      wsi_x11_get_connection(wsi_device, alloc, conn);
> +      wsi_x11_get_connection(wsi_device, conn);
>     if (!wsi_conn)
>        return VK_ERROR_OUT_OF_HOST_MEMORY;
>  
> @@ -1279,7 +1276,7 @@ x11_surface_create_swapchain(VkIcdSurfaceBase *icd_surface,
>  
>     xcb_connection_t *conn = x11_surface_get_connection(icd_surface);
>     struct wsi_x11_connection *wsi_conn =
> -      wsi_x11_get_connection(wsi_device, pAllocator, conn);
> +      wsi_x11_get_connection(wsi_device, conn);
>     if (!wsi_conn)
>        return VK_ERROR_OUT_OF_HOST_MEMORY;
>  
> @@ -1501,7 +1498,7 @@ wsi_x11_finish_wsi(struct wsi_device *wsi_device,
>     if (wsi) {
>        struct hash_entry *entry;
>        hash_table_foreach(wsi->connections, entry)
> -         wsi_x11_connection_destroy(alloc, entry->data);
> +         wsi_x11_connection_destroy(wsi_device, entry->data);
>  
>        _mesa_hash_table_destroy(wsi->connections, NULL);
>  
> diff --git a/src/vulkan/wsi/wsi_common_x11.h b/src/vulkan/wsi/wsi_common_x11.h
> index b33540856cb..e9b3ecfb2e0 100644
> --- a/src/vulkan/wsi/wsi_common_x11.h
> +++ b/src/vulkan/wsi/wsi_common_x11.h
> @@ -27,7 +27,6 @@
>  
>  VkBool32 wsi_get_physical_device_xcb_presentation_support(
>      struct wsi_device *wsi_device,
> -    VkAllocationCallbacks *alloc,
>      uint32_t                                    queueFamilyIndex,
>      int local_fd,
>      bool can_handle_different_gpu,
> -- 
> 2.19.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list