Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Logger callback function sink #1748

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+1,630 −362
Open

Logger callback function sink #1748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4abd3e0
Added new output sink to the logger which is a user configurable call…
martygrant Jun 10, 2024
02c0aa0
Apply suggestions from code review
martygrant Jul 4, 2024
8936426
Rerun generate target after applying suggestions from Github web ui. …
martygrant Jul 5, 2024
e592c3d
Update tracing mako template to account to use new logger level enum.
martygrant Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 107 additions & 21 deletions include/ur_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,28 +208,30 @@ typedef enum ur_function_t {
UR_FUNCTION_COMMAND_BUFFER_GET_INFO_EXP = 221, ///< Enumerator for ::urCommandBufferGetInfoExp
UR_FUNCTION_COMMAND_BUFFER_COMMAND_GET_INFO_EXP = 222, ///< Enumerator for ::urCommandBufferCommandGetInfoExp
UR_FUNCTION_ENQUEUE_TIMESTAMP_RECORDING_EXP = 223, ///< Enumerator for ::urEnqueueTimestampRecordingExp
UR_FUNCTION_ENQUEUE_KERNEL_LAUNCH_CUSTOM_EXP = 224, ///< Enumerator for ::urEnqueueKernelLaunchCustomExp
UR_FUNCTION_KERNEL_GET_SUGGESTED_LOCAL_WORK_SIZE = 225, ///< Enumerator for ::urKernelGetSuggestedLocalWorkSize
UR_FUNCTION_BINDLESS_IMAGES_IMPORT_EXTERNAL_MEMORY_EXP = 226, ///< Enumerator for ::urBindlessImagesImportExternalMemoryExp
UR_FUNCTION_BINDLESS_IMAGES_IMPORT_EXTERNAL_SEMAPHORE_EXP = 227, ///< Enumerator for ::urBindlessImagesImportExternalSemaphoreExp
UR_FUNCTION_ENQUEUE_NATIVE_COMMAND_EXP = 228, ///< Enumerator for ::urEnqueueNativeCommandExp
UR_FUNCTION_LOADER_CONFIG_SET_MOCKING_ENABLED = 229, ///< Enumerator for ::urLoaderConfigSetMockingEnabled
UR_FUNCTION_LOADER_CONFIG_SET_MOCKING_ENABLED = 224, ///< Enumerator for ::urLoaderConfigSetMockingEnabled
UR_FUNCTION_ADAPTER_SET_LOGGER_CALLBACK = 225, ///< Enumerator for ::urAdapterSetLoggerCallback
UR_FUNCTION_ADAPTER_SET_LOGGER_CALLBACK_LEVEL = 226, ///< Enumerator for ::urAdapterSetLoggerCallbackLevel
UR_FUNCTION_KERNEL_GET_SUGGESTED_LOCAL_WORK_SIZE = 227, ///< Enumerator for ::urKernelGetSuggestedLocalWorkSize
UR_FUNCTION_BINDLESS_IMAGES_IMPORT_EXTERNAL_MEMORY_EXP = 228, ///< Enumerator for ::urBindlessImagesImportExternalMemoryExp
UR_FUNCTION_BINDLESS_IMAGES_MAP_EXTERNAL_LINEAR_MEMORY_EXP = 229, ///< Enumerator for ::urBindlessImagesMapExternalLinearMemoryExp
UR_FUNCTION_BINDLESS_IMAGES_RELEASE_EXTERNAL_MEMORY_EXP = 230, ///< Enumerator for ::urBindlessImagesReleaseExternalMemoryExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_USM_MEMCPY_EXP = 231, ///< Enumerator for ::urCommandBufferAppendUSMMemcpyExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_USM_FILL_EXP = 232, ///< Enumerator for ::urCommandBufferAppendUSMFillExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_COPY_EXP = 233, ///< Enumerator for ::urCommandBufferAppendMemBufferCopyExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_WRITE_EXP = 234, ///< Enumerator for ::urCommandBufferAppendMemBufferWriteExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_READ_EXP = 235, ///< Enumerator for ::urCommandBufferAppendMemBufferReadExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_COPY_RECT_EXP = 236, ///< Enumerator for ::urCommandBufferAppendMemBufferCopyRectExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_WRITE_RECT_EXP = 237, ///< Enumerator for ::urCommandBufferAppendMemBufferWriteRectExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_READ_RECT_EXP = 238, ///< Enumerator for ::urCommandBufferAppendMemBufferReadRectExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_FILL_EXP = 239, ///< Enumerator for ::urCommandBufferAppendMemBufferFillExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_USM_PREFETCH_EXP = 240, ///< Enumerator for ::urCommandBufferAppendUSMPrefetchExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_USM_ADVISE_EXP = 241, ///< Enumerator for ::urCommandBufferAppendUSMAdviseExp
UR_FUNCTION_COMMAND_BUFFER_ENQUEUE_EXP = 242, ///< Enumerator for ::urCommandBufferEnqueueExp
UR_FUNCTION_COMMAND_BUFFER_UPDATE_SIGNAL_EVENT_EXP = 243, ///< Enumerator for ::urCommandBufferUpdateSignalEventExp
UR_FUNCTION_COMMAND_BUFFER_UPDATE_WAIT_EVENTS_EXP = 244, ///< Enumerator for ::urCommandBufferUpdateWaitEventsExp
UR_FUNCTION_BINDLESS_IMAGES_MAP_EXTERNAL_LINEAR_MEMORY_EXP = 245, ///< Enumerator for ::urBindlessImagesMapExternalLinearMemoryExp
UR_FUNCTION_BINDLESS_IMAGES_IMPORT_EXTERNAL_SEMAPHORE_EXP = 231, ///< Enumerator for ::urBindlessImagesImportExternalSemaphoreExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_USM_MEMCPY_EXP = 232, ///< Enumerator for ::urCommandBufferAppendUSMMemcpyExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_USM_FILL_EXP = 233, ///< Enumerator for ::urCommandBufferAppendUSMFillExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_COPY_EXP = 234, ///< Enumerator for ::urCommandBufferAppendMemBufferCopyExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_WRITE_EXP = 235, ///< Enumerator for ::urCommandBufferAppendMemBufferWriteExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_READ_EXP = 236, ///< Enumerator for ::urCommandBufferAppendMemBufferReadExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_COPY_RECT_EXP = 237, ///< Enumerator for ::urCommandBufferAppendMemBufferCopyRectExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_WRITE_RECT_EXP = 238, ///< Enumerator for ::urCommandBufferAppendMemBufferWriteRectExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_READ_RECT_EXP = 239, ///< Enumerator for ::urCommandBufferAppendMemBufferReadRectExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_MEM_BUFFER_FILL_EXP = 240, ///< Enumerator for ::urCommandBufferAppendMemBufferFillExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_USM_PREFETCH_EXP = 241, ///< Enumerator for ::urCommandBufferAppendUSMPrefetchExp
UR_FUNCTION_COMMAND_BUFFER_APPEND_USM_ADVISE_EXP = 242, ///< Enumerator for ::urCommandBufferAppendUSMAdviseExp
UR_FUNCTION_COMMAND_BUFFER_ENQUEUE_EXP = 243, ///< Enumerator for ::urCommandBufferEnqueueExp
UR_FUNCTION_COMMAND_BUFFER_UPDATE_SIGNAL_EVENT_EXP = 244, ///< Enumerator for ::urCommandBufferUpdateSignalEventExp
UR_FUNCTION_COMMAND_BUFFER_UPDATE_WAIT_EVENTS_EXP = 245, ///< Enumerator for ::urCommandBufferUpdateWaitEventsExp
UR_FUNCTION_ENQUEUE_KERNEL_LAUNCH_CUSTOM_EXP = 246, ///< Enumerator for ::urEnqueueKernelLaunchCustomExp
UR_FUNCTION_ENQUEUE_NATIVE_COMMAND_EXP = 247, ///< Enumerator for ::urEnqueueNativeCommandExp
/// @cond
UR_FUNCTION_FORCE_UINT32 = 0x7fffffff
/// @endcond
Expand Down Expand Up @@ -1025,6 +1027,70 @@ typedef enum ur_adapter_backend_t {

} ur_adapter_backend_t;

///////////////////////////////////////////////////////////////////////////////
/// @brief Minimum level of messages to be processed by the logger.
typedef enum ur_logger_level_t {
UR_LOGGER_LEVEL_DEBUG = 0, ///< Debugging messages used for development purposes.
UR_LOGGER_LEVEL_INFO = 1, ///< General messages not related to debugging, warnings or errors.
UR_LOGGER_LEVEL_WARN = 2, ///< Used to warn users about potential problems.
UR_LOGGER_LEVEL_ERR = 3, ///< Used when an error has occurred.
UR_LOGGER_LEVEL_QUIET = 4, ///< Restrict logger processing any messages.
/// @cond
UR_LOGGER_LEVEL_FORCE_UINT32 = 0x7fffffff
/// @endcond

} ur_logger_level_t;

///////////////////////////////////////////////////////////////////////////////
/// @brief Callback function to retrieve output from the logger.
typedef void (*ur_logger_callback_t)(
ur_logger_level_t level, ///< [out] Minimum level of messages to be processed by the logger.
const char *pLoggerMsg, ///< [in][out] pointer to data to be passed to callback
void *pUserData ///< [in][out] pointer to data to be passed to callback
);

///////////////////////////////////////////////////////////////////////////////
/// @brief Set a callback function for use by the logger to retrieve logging output.
/// It is a requirement that the callback function is thread safe and the
/// creator of the function will be responsible for this.
///
Copy link
Contributor

@pbalcer pbalcer Jul 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing to document is that calling this, urAdapterSetLoggerCallback, and urAdapterSetLoggerCallbackLevel functions is not thread-safe with respect to any other function in the API. The reason is that they are modifying global state without using atomics or grabbing a lock. In other words, when something is writing a log, you may inadvertently change the logger callback pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay no problem, I reworded the description for urAdapterSetLoggerCallback but will add it to urAdapterSetLoggerCallbackLevel as well. It should specifically say not "thread safe with respect to any other function in the API"? Should the explanation be added here or is that enough?

Copy link
Contributor

@pbalcer pbalcer Jul 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most people understand thread-safe functions as safe to be invoked, with respect to themselves, concurrently by multiple threads. And that is how POSIX defines it. For example, you can't call strtok() from multiple threads and expect it to work correctly, but you can call strtok() in one thread, and e.g., write() in another. In this case, the situation is a bit different, since urAdapterSetLoggerCallback is not safe to call even with respect to other UR API functions (that are otherwise thread-safe). Small caveat is that implementations may choose to limit this unsafety to be per-adapter.

In my opinion this break the principle of least surprise, and should be thoroughly documented. Saying "not thread-safe" is not enough.

Ideally we'd fix this - one approach would be to only allow setting the callback as part of the loader config (and then somehow pass that callback to the adapter during initialization... won't be trivial), and then change the logger level variable to be an atomic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Piotr. @kbenzie do you have any thoughts on this? Would we want to modify the design now or just get this PR in as it is now and modify it later? If the latter then I'll move these comments to a new issue to track this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets revisit making this decision until after intel/llvm#14145 is merged.

/// @returns
/// - ::UR_RESULT_SUCCESS
/// - ::UR_RESULT_ERROR_UNINITIALIZED
/// - ::UR_RESULT_ERROR_DEVICE_LOST
/// - ::UR_RESULT_ERROR_ADAPTER_SPECIFIC
/// - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
/// + `NULL == hAdapter`
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
/// + `NULL == pfnLoggerCallback`
/// - ::UR_RESULT_ERROR_INVALID_ENUMERATION
/// + `::UR_LOGGER_LEVEL_QUIET < level`
UR_APIEXPORT ur_result_t UR_APICALL
urAdapterSetLoggerCallback(
ur_adapter_handle_t hAdapter, ///< [in] handle of the adapter
ur_logger_callback_t pfnLoggerCallback, ///< [in] Function pointer to callback from the logger.
void *pUserData, ///< [in][out][optional] pointer to data to be passed to callback
ur_logger_level_t level ///< [in] logging level
);

///////////////////////////////////////////////////////////////////////////////
/// @brief Set the minimum logging level for the logger Callback function.
///
/// @returns
/// - ::UR_RESULT_SUCCESS
/// - ::UR_RESULT_ERROR_UNINITIALIZED
/// - ::UR_RESULT_ERROR_DEVICE_LOST
/// - ::UR_RESULT_ERROR_ADAPTER_SPECIFIC
/// - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
/// + `NULL == hAdapter`
/// - ::UR_RESULT_ERROR_INVALID_ENUMERATION
/// + `::UR_LOGGER_LEVEL_QUIET < level`
UR_APIEXPORT ur_result_t UR_APICALL
urAdapterSetLoggerCallbackLevel(
ur_adapter_handle_t hAdapter, ///< [in] handle of the adapter
ur_logger_level_t level ///< [in] logging level
);

#if !defined(__GNUC__)
#pragma endregion
#endif
Expand Down Expand Up @@ -10087,6 +10153,26 @@ typedef struct ur_loader_config_set_mocking_enabled_params_t {
ur_bool_t *penable;
} ur_loader_config_set_mocking_enabled_params_t;

///////////////////////////////////////////////////////////////////////////////
/// @brief Function parameters for urAdapterSetLoggerCallback
/// @details Each entry is a pointer to the parameter passed to the function;
/// allowing the callback the ability to modify the parameter's value
typedef struct ur_adapter_set_logger_callback_params_t {
ur_adapter_handle_t *phAdapter;
ur_logger_callback_t *ppfnLoggerCallback;
void **ppUserData;
ur_logger_level_t *plevel;
} ur_adapter_set_logger_callback_params_t;

///////////////////////////////////////////////////////////////////////////////
/// @brief Function parameters for urAdapterSetLoggerCallbackLevel
/// @details Each entry is a pointer to the parameter passed to the function;
/// allowing the callback the ability to modify the parameter's value
typedef struct ur_adapter_set_logger_callback_level_params_t {
ur_adapter_handle_t *phAdapter;
ur_logger_level_t *plevel;
} ur_adapter_set_logger_callback_level_params_t;

///////////////////////////////////////////////////////////////////////////////
/// @brief Function parameters for urPlatformGet
/// @details Each entry is a pointer to the parameter passed to the function;
Expand Down
2 changes: 2 additions & 0 deletions include/ur_api_funcs.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

// Auto-generated file, do not edit.

_UR_API(urAdapterSetLoggerCallback)
_UR_API(urAdapterSetLoggerCallbackLevel)
_UR_API(urPlatformGet)
_UR_API(urPlatformGetInfo)
_UR_API(urPlatformGetNativeHandle)
Expand Down
43 changes: 43 additions & 0 deletions include/ur_ddi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,48 @@
extern "C" {
#endif

///////////////////////////////////////////////////////////////////////////////
/// @brief Function-pointer for urAdapterSetLoggerCallback
typedef ur_result_t(UR_APICALL *ur_pfnAdapterSetLoggerCallback_t)(
ur_adapter_handle_t,
ur_logger_callback_t,
void *,
ur_logger_level_t);

///////////////////////////////////////////////////////////////////////////////
/// @brief Function-pointer for urAdapterSetLoggerCallbackLevel
typedef ur_result_t(UR_APICALL *ur_pfnAdapterSetLoggerCallbackLevel_t)(
ur_adapter_handle_t,
ur_logger_level_t);

///////////////////////////////////////////////////////////////////////////////
/// @brief Table of Adapter functions pointers
typedef struct ur_adapter_dditable_t {
ur_pfnAdapterSetLoggerCallback_t pfnSetLoggerCallback;
ur_pfnAdapterSetLoggerCallbackLevel_t pfnSetLoggerCallbackLevel;
} ur_adapter_dditable_t;

///////////////////////////////////////////////////////////////////////////////
/// @brief Exported function for filling application's Adapter table
/// with current process' addresses
///
/// @returns
/// - ::UR_RESULT_SUCCESS
/// - ::UR_RESULT_ERROR_UNINITIALIZED
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
/// - ::UR_RESULT_ERROR_UNSUPPORTED_VERSION
UR_DLLEXPORT ur_result_t UR_APICALL
urGetAdapterProcAddrTable(
ur_api_version_t version, ///< [in] API version requested
ur_adapter_dditable_t *pDdiTable ///< [in,out] pointer to table of DDI function pointers
);

///////////////////////////////////////////////////////////////////////////////
/// @brief Function-pointer for urGetAdapterProcAddrTable
typedef ur_result_t(UR_APICALL *ur_pfnGetAdapterProcAddrTable_t)(
ur_api_version_t,
ur_adapter_dditable_t *);

///////////////////////////////////////////////////////////////////////////////
/// @brief Function-pointer for urPlatformGet
typedef ur_result_t(UR_APICALL *ur_pfnPlatformGet_t)(
Expand Down Expand Up @@ -2486,6 +2528,7 @@ typedef ur_result_t(UR_APICALL *ur_pfnGetDeviceProcAddrTable_t)(
///////////////////////////////////////////////////////////////////////////////
/// @brief Container for all DDI tables
typedef struct ur_dditable_t {
ur_adapter_dditable_t Adapter;
ur_platform_dditable_t Platform;
ur_context_dditable_t Context;
ur_event_dditable_t Event;
Expand Down
24 changes: 24 additions & 0 deletions include/ur_print.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urPrintAdapterInfo(enum ur_adapter_info_t va
/// - `buff_size < out_size`
UR_APIEXPORT ur_result_t UR_APICALL urPrintAdapterBackend(enum ur_adapter_backend_t value, char *buffer, const size_t buff_size, size_t *out_size);

///////////////////////////////////////////////////////////////////////////////
/// @brief Print ur_logger_level_t enum
/// @returns
/// - ::UR_RESULT_SUCCESS
/// - ::UR_RESULT_ERROR_INVALID_SIZE
/// - `buff_size < out_size`
UR_APIEXPORT ur_result_t UR_APICALL urPrintLoggerLevel(enum ur_logger_level_t value, char *buffer, const size_t buff_size, size_t *out_size);

///////////////////////////////////////////////////////////////////////////////
/// @brief Print ur_platform_info_t enum
/// @returns
Expand Down Expand Up @@ -1130,6 +1138,22 @@ UR_APIEXPORT ur_result_t UR_APICALL urPrintLoaderConfigSetCodeLocationCallbackPa
/// - `buff_size < out_size`
UR_APIEXPORT ur_result_t UR_APICALL urPrintLoaderConfigSetMockingEnabledParams(const struct ur_loader_config_set_mocking_enabled_params_t *params, char *buffer, const size_t buff_size, size_t *out_size);

///////////////////////////////////////////////////////////////////////////////
/// @brief Print ur_adapter_set_logger_callback_params_t struct
/// @returns
/// - ::UR_RESULT_SUCCESS
/// - ::UR_RESULT_ERROR_INVALID_SIZE
/// - `buff_size < out_size`
UR_APIEXPORT ur_result_t UR_APICALL urPrintAdapterSetLoggerCallbackParams(const struct ur_adapter_set_logger_callback_params_t *params, char *buffer, const size_t buff_size, size_t *out_size);

///////////////////////////////////////////////////////////////////////////////
/// @brief Print ur_adapter_set_logger_callback_level_params_t struct
/// @returns
/// - ::UR_RESULT_SUCCESS
/// - ::UR_RESULT_ERROR_INVALID_SIZE
/// - `buff_size < out_size`
UR_APIEXPORT ur_result_t UR_APICALL urPrintAdapterSetLoggerCallbackLevelParams(const struct ur_adapter_set_logger_callback_level_params_t *params, char *buffer, const size_t buff_size, size_t *out_size);

///////////////////////////////////////////////////////////////////////////////
/// @brief Print ur_platform_get_params_t struct
/// @returns
Expand Down
Loading
Loading