Skip to content

Commit 4a912ed

Browse files
[SYCL][Bindless][UR][L0][E2E] Fix linear interop memory and L0 V1 adapter leaks. (#18353)
This patch fixes the mapping of interop memory to a linear range represented by a `void*`. The SYCL Bindless Images spec is clarified with wording to indicate which free APIs should be used on mapped imported memory. Using `sycl::free` for freeing mapped imported linear memory regions does not work for the LevelZero adapter, as the backend needs to call `zeMemFree` directly - it cannot go through the regular `sycl::free` code as that will use UMF free functions by default. This is invalid for memory regions mapped using `map_external_linear_memory`. The `unmap_external_linear_memory` SYCL API has been added to the Bindless Images specification to remedy this problem. An `unmap_external_image_memory` has also been added. This patch also introduces an E2E test which verifies that a Vulkan buffer's memory can be correctly imported into SYCL and its data retrieved from and written to within a SYCL kernel by using purely USM pointers (i.e. without using bindless images). This is a step towards extending the test coverage of interop functionality currently in the Bindless Images extension, with the goal being at some point splitting the interop functionality into one or more separate extensions. Two other fixes have been made in the LevelZero UR adapter. A leaked `ze_image_handle_t` object has been fixed in the LevelZero V1 adapter. The previous implementation incorrectly reinterpreted bindless memory handles as `ur_mem_handle_t` types. The implementation of `urBindlessImagesImageFreeExp` is now unified between V1 and V2, removing the leak. As such the `level_zero/v2/image.cpp` file has been removed. The `urMemoryHandle` member of `ur_ze_external_memory_data` has been removed. Aside from being incorrectly reinterpreted from `ur_bindless_mem_handle_t`, the LevelZero adapter does not need to track the memory handle for external memory resources.
1 parent a6fbb40 commit 4a912ed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+937
-82
lines changed

sycl/doc/extensions/experimental/sycl_ext_oneapi_bindless_images.asciidoc

+38-7
Original file line numberDiff line numberDiff line change
@@ -2150,6 +2150,10 @@ the same way as memory allocated through `alloc_image_mem`,
21502150
operations also work with imported memory mapped to `image_mem_handle` and
21512151
`void *` types.
21522152
2153+
An `external_mem` handle can only be mapped to a single `image_mem_handle` or
2154+
`void *` at any one time. Attempting to map a single `external_mem` handle to
2155+
more than one `image_mem_handle` or `void *` is considered undefined behaviour.
2156+
21532157
When calling `create_image` with an `image_mem_handle` or `void *` mapped from
21542158
an external memory object, the user must ensure that the image descriptor they
21552159
pass to `create_image` has members that match or map to those of the external
@@ -2165,11 +2169,41 @@ the external API. The current supported importable image types are `standard`
21652169
and `mipmap`. Attempting to import other image types will result in undefined
21662170
behaviour.
21672171
2168-
Once a user has finished operating on imported memory, they must ensure that
2169-
they destroy the imported memory handle through `release_external_memory`.
2172+
Once a user has finished operating on mapped memory, they must ensure that they
2173+
unmap that memory.
2174+
2175+
Memory mapped using `map_external_image_memory` should be unmapped using
2176+
`unmap_external_image_memory`. The `image_type` parameter passed to this
2177+
function must reflect the `image_type` of the image descriptor used when the
2178+
memory was originally mapped. Passing an `image_type` value different to that of
2179+
the value used in the image descriptor when the memory was originally mapped
2180+
will result in undefined behaviour.
21702181
2171-
`release_external_memory` can only accept `external_mem` objects that were
2172-
created through `import_external_memory`.
2182+
Memory mapped using `map_external_linear_memory` should be unmapped using
2183+
`unmap_external_linear_memory`.
2184+
2185+
```cpp
2186+
namespace sycl::ext::oneapi::experimental {
2187+
2188+
void unmap_external_image_memory(image_mem_handle mappedImageMem,
2189+
image_type imageType,
2190+
const sycl::device &syclDevice,
2191+
const sycl::context &syclContext);
2192+
void unmap_external_image_memory(image_mem_handle mappedImageMem,
2193+
image_type imageType,
2194+
const sycl::queue &syclQueue);
2195+
2196+
void unmap_external_linear_memory(void *mappedLinearMem,
2197+
const sycl::device &syclDevice,
2198+
const sycl::context &syclContext);
2199+
void unmap_external_linear_memory(void *mappedLinearMem,
2200+
const sycl::queue &syclQueue);
2201+
}
2202+
```
2203+
2204+
Once all memory mapped from a given `external_mem` handle has been unmapped,
2205+
and the user has finished operating on the external memory, they should then
2206+
release the `externa_mem` handle using `release_external_memory`.
21732207
21742208
```cpp
21752209
namespace sycl::ext::oneapi::experimental {
@@ -2182,9 +2216,6 @@ void release_external_memory(external_mem externalMem,
21822216
}
21832217
```
21842218
2185-
Destroying or freeing any imported memory through `image_mem_free` or
2186-
`sycl::free` will result in undefined behavior.
2187-
21882219
=== Importing external semaphores [[importing_external_semaphores]]
21892220
21902221
In addition to proposing importation of external memory resources, we also

sycl/include/sycl/ext/oneapi/bindless_images.hpp

+48
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,54 @@ __SYCL_EXPORT void release_external_memory(external_mem externalMem,
285285
__SYCL_EXPORT void release_external_memory(external_mem externalMem,
286286
const sycl::queue &syclQueue);
287287

288+
/**
289+
* @brief Unmap external linear memory region
290+
*
291+
* @param mappedLinearMem Pointer to the mapped linear memory region to unmap
292+
* @param syclDevice The device in which the external memory was created
293+
* @param syclContext The context in which the external memory was created
294+
*/
295+
__SYCL_EXPORT void
296+
unmap_external_linear_memory(void *mappedLinearMem,
297+
const sycl::device &syclDevice,
298+
const sycl::context &syclContext);
299+
300+
/**
301+
* @brief Unmap external linear memory region
302+
*
303+
* @param mappedLinearMem Pointer to the mapped linear memory region to unmap
304+
* @param syclQueue The queue in which the external memory was created
305+
*/
306+
inline void unmap_external_linear_memory(void *mappedLinearMem,
307+
const sycl::queue &syclQueue) {
308+
unmap_external_linear_memory(mappedLinearMem, syclQueue.get_device(),
309+
syclQueue.get_context());
310+
}
311+
312+
/**
313+
* @brief Unmap external image memory
314+
*
315+
* @param mappedImageMem Handle to the mapped image memory to unmap
316+
* @param syclDevice The device in which the external memory was created
317+
* @param syclContext The context in which the external memory was created
318+
*/
319+
__SYCL_EXPORT void unmap_external_image_memory(
320+
image_mem_handle mappedImageMem, image_type imageType,
321+
const sycl::device &syclDevice, const sycl::context &syclContext);
322+
323+
/**
324+
* @brief Unmap external image memory
325+
*
326+
* @param mappedImageMem Handle to the mapped image memory to unmap
327+
* @param syclQueue The queue in which the external memory was created
328+
*/
329+
inline void unmap_external_image_memory(image_mem_handle mappedImageMem,
330+
image_type imageType,
331+
const sycl::queue &syclQueue) {
332+
unmap_external_image_memory(mappedImageMem, imageType, syclQueue.get_device(),
333+
syclQueue.get_context());
334+
}
335+
288336
/**
289337
* @brief Create an image and return the device image handle
290338
*

sycl/source/detail/bindless_images.cpp

+18
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,24 @@ __SYCL_EXPORT void release_external_memory(external_mem extMem,
546546
syclQueue.get_context());
547547
}
548548

549+
__SYCL_EXPORT void
550+
unmap_external_linear_memory(void *mappedLinearRegion,
551+
const sycl::device &syclDevice,
552+
const sycl::context &syclContext) {
553+
auto [urDevice, urCtx, Adapter] = get_ur_handles(syclDevice, syclContext);
554+
555+
Adapter->call<
556+
sycl::errc::invalid,
557+
sycl::detail::UrApiKind::urBindlessImagesFreeMappedLinearMemoryExp>(
558+
urCtx, urDevice, mappedLinearRegion);
559+
}
560+
561+
__SYCL_EXPORT void unmap_external_image_memory(
562+
image_mem_handle mappedImageMem, image_type imageType,
563+
const sycl::device &syclDevice, const sycl::context &syclContext) {
564+
free_image_mem(mappedImageMem, imageType, syclDevice, syclContext);
565+
}
566+
549567
template <>
550568
__SYCL_EXPORT external_semaphore import_external_semaphore(
551569
external_semaphore_descriptor<resource_fd> externalSemaphoreDesc,

0 commit comments

Comments
 (0)