The last CTL patch :) #1524

lplewa · 2025-09-17T16:48:43Z

No description provided.

Split between initialize and post-initialize function is necessary for properly handling CTL defaults.

lukaszstolarczuk

after all, this is super huge PR - I'd split this somehow, if easily done...

.github/workflows/reusable_compatibility.yml

examples/ctl/ctl_example.c

examples/README.md

include/umf/memory_provider_ops.h

test/ctl/ctl_env_disjoint_pool.cfg

docs/config/examples.rst

docs/config/ctl.rst

defaults changes global state, so using forks will isolate tests

lukaszstolarczuk · 2025-10-03T11:21:46Z

README.md


+UMF's experimental CTL API is showcased in the [CTL example](examples/ctl/ctl.c),
+which explores provider and pool statistics, and in the [custom CTL example](examples/ctl/custom_ctl.c), which wires CTL support into a bespoke memory
+provider. These examples rely on experimental headers whose interfaces may


whose refer to people -> perhaps: ... experimental headers which may change ...

lukaszstolarczuk · 2025-10-03T13:07:26Z

docs/config/ctl.rst

+                sizeof(alloc_count), name, i);
+  }
+
+Ensure that the types of wildcard arguments match the expected node types.


otherwise what? you get an error, a crash, ...?

undefined behavior may happen

undefined by C standard not our code.

lukaszstolarczuk · 2025-10-03T13:08:33Z

docs/config/ctl.rst

+            sizeof(capacity));
+
+Every subsequently created disjoint pool will use ``16`` as its starting
+capacity overriding it's creation parameters. Defaults are keyed by the


it's -> its

lukaszstolarczuk · 2025-10-03T13:09:54Z

docs/config/ctl.rst

+
+  UMF_CONF="umf.logger.output=stdout;umf.logger.level=0"
+
+CTL options available through environment variables are limited—you can only


limited—you misspell

lukaszstolarczuk · 2025-10-03T13:44:32Z

docs/config/ctl.rst

+Within each subsystem the path continues with an addressing scheme followed by
+the module or leaf of interest.
+
+Reading this reference


consider renaming this section to Reading below sections or dunno, something else ;d

lukaszstolarczuk · 2025-10-03T13:46:06Z

docs/config/ctl.rst

+:type:`umf_memory_provider_handle_t` argument to reach a specific provider.
+Providers can also be addressed by name through ``umf.provider.by_name.{provider}``;
+append ``.{index}`` to address specific provider when multiple providers share the same label.
+Defaults for future providers reside under ``umf.provider.default.{provider}`` and track the


not sure what you meant by ... and track the name ...

lukaszstolarczuk · 2025-10-03T13:58:08Z

docs/config/ctl.rst

+===========
+
+A ``{}`` in the path acts as a wildcard and is replaced with successive
+arguments of ``umfCtlGet``, ``umfCtlSet`` or ``umfCtlExec``. Wildcards can


set and get are explained, but umfCtlExec wasn't

TBH we do not explain set or get either. Do we need such section?

lukaszstolarczuk · 2025-10-03T14:04:22Z

examples/ctl/CMakeLists.txt

+target_include_directories(${ EXAMPLE_NAME} PRIVATE ${ LIBUMF_INCLUDE_DIRS})
+target_link_directories(
+    ${
+    EXAMPLE_NAME}


that's a weird formatting - can you make it like ${EXAMPLE_NAME}...?

// fix below, as well pls

not my faults - this is done by cmake format

bratpiorka · 2025-10-07T15:19:47Z

README.md

 There are also more advanced examples that allocate USM memory from the [Level Zero device](examples/level_zero_shared_memory/level_zero_shared_memory.c) using the Level Zero API and UMF Level Zero memory provider and [CUDA device](examples/cuda_shared_memory/cuda_shared_memory.c) using the CUDA API and UMF CUDA memory provider.

+UMF's experimental CTL API is showcased in the [CTL example](examples/ctl/ctl.c),
+which explores provider and pool statistics, and in the [custom CTL example](examples/ctl/custom_ctl.c), which wires CTL support into a bespoke memory


bespoke -> customized

bratpiorka · 2025-10-07T15:21:25Z

docs/config/examples.rst

+UMF repository.
+
+The sample configures an OS memory provider and a disjoint pool, reuses the
+provider's canonical ``OS`` selector obtained at runtime, assigns a custom pool


cannonical?

bratpiorka · 2025-10-07T15:21:49Z

docs/config/examples.rst

+name, and then mixes ``by_handle`` and ``by_name`` selectors to explore CTL
+statistics. Wildcard nodes are used to choose provider counters, build a
+four-segment ``{}.{}`` chain for the named pool, reset the peak tracker, and
+drill into per-bucket disjoint pool telemetry. The program prints hints on


bratpiorka · 2025-10-07T15:23:51Z

examples/README.md

+
+This example configures an OS memory provider and disjoint pool, then queries
+statistics through CTL using both ``by_handle`` and ``by_name`` selectors. It
+demonstrates wildcard nodes to mix selectors, reset peak counters, and drill


drill into?

bratpiorka · 2025-10-07T15:25:27Z

examples/ctl/CMakeLists.txt

+add_executable(${ EXAMPLE_NAME} custom_ctl.c)
+target_include_directories(${ EXAMPLE_NAME} PRIVATE ${ LIBUMF_INCLUDE_DIRS})
+target_link_directories(
+    ${


fix formatting

bratpiorka · 2025-10-07T15:26:43Z

examples/ctl/CMakeLists.txt

+        TEST ${ EXAMPLE_NAME}
+        PROPERTY ENVIRONMENT_MODIFICATION
+                 "LD_LIBRARY_PATH=path_list_append:"
+                 "${LIBUMF_LIBRARY_DIRS};LD_"


do not break "LD_LIBRARY..."

bratpiorka · 2025-10-07T15:28:36Z

examples/ctl/ctl.c

+ *
+ */
+
+#include "umf/base.h"


use <> for system headers, group and change order

bratpiorka · 2025-10-07T15:31:40Z

src/ctl/ctl_defaults.c

+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ */
+
+#include "ctl_defaults.h"


move down to other internal includes

bratpiorka · 2025-10-08T08:21:28Z

src/pool/pool_disjoint.c


 static char *DEFAULT_NAME = "disjoint";

+enum {


please add a comment how this is used
as I understand you create single bitmask instead of multiple bool flags .. maybe you could keep this enum close to "params_overriden" filed?

bratpiorka · 2025-10-08T08:24:04Z

src/pool/pool_disjoint.c

 static char *DEFAULT_NAME = "disjoint";

+enum {
+    DP_OVERRIDE_SLAB_MIN_SIZE = 1 << 0,


if you initialize the bitmask to 0 does it mean that by default slab min size is always overriden?

Copilot

Pull Request Overview

This PR consolidates final CTL (Control Interface) changes for version 1.0, including error code adjustments and comprehensive implementation of CTL functionality for providers and pools. The CTL system provides a hierarchical interface for configuring and querying UMF components.

Key changes:

Introduced new error code UMF_RESULT_ERROR_INVALID_CTL_PATH for invalid CTL paths
Added comprehensive CTL parameter support for disjoint pools with default overrides
Enhanced provider and pool initialization with post-initialization hooks
Added extensive test coverage and examples for CTL functionality

Reviewed Changes

Copilot reviewed 49 out of 49 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`include/umf/base.h`	Added new error code `UMF_RESULT_ERROR_INVALID_CTL_PATH`
`test/utils/cpp_helpers.hpp`	Updated default error return from `UMF_RESULT_ERROR_UNKNOWN` to `UMF_RESULT_ERROR_INVALID_CTL_PATH`
`test/pools/disjoint_pool_ctl.cpp`	Added comprehensive CTL tests for disjoint pool parameters and defaults
`test/pools/disjoint_pool.cpp`	Added post-initialization CTL calls to existing tests
`src/pool/pool_disjoint.c`	Implemented extensive CTL parameter support with read/write handlers
`src/memory_provider.c`	Added provider CTL defaults system and post-initialization support
`src/memory_pool.c`	Enhanced pool creation with CTL defaults and post-initialization
`examples/ctl/`	Added comprehensive CTL usage examples

Copilot · 2025-10-08T08:38:19Z

test/pools/disjoint_pool_ctl.cpp

    ret = umfCtlGet("umf.pool.by_handle.{}.stats.buckets.count", &arg, 1,
                    poolWrapper.get());
-    EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+    EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_CTL_PATH);


The buffer size is too small (1 byte) for a size_t value. This should be sizeof(arg) to match the expected data type size.

Copilot · 2025-10-08T08:38:19Z

test/pools/disjoint_pool_ctl.cpp

    ret = umfCtlGet("umf.pool.by_handle.{}.stats.1.alloc_num", &arg, 1,
                    poolWrapper.get());
-    EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+    EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_CTL_PATH);


The buffer size is too small (1 byte) for a size_t value. This should be sizeof(arg) to match the expected data type size.

Copilot · 2025-10-08T08:38:20Z

test/pools/disjoint_pool_ctl.cpp


    // no bucket id
    ret = umfCtlGet("umf.pool.by_handle.{}.stats.buckets.alloc_num", &arg,
                    sizeof(arg), poolWrapper.get());


The buffer size is inconsistent with other similar test cases. For consistency with the test pattern, this should use 1 as the buffer size to test the invalid path scenario.

Suggested change

sizeof(arg), poolWrapper.get());

1, poolWrapper.get());

Copilot · 2025-10-08T08:38:20Z

test/pools/disjoint_pool_ctl.cpp


    // bucked id + count
    ret = umfCtlGet("umf.pool.by_handle.{}.stats.buckets.1.count", &arg,
                    sizeof(arg), poolWrapper.get());


The buffer size is inconsistent with other similar test cases. For consistency with the test pattern, this should use 1 as the buffer size to test the invalid path scenario.

Suggested change

sizeof(arg), poolWrapper.get());

1, poolWrapper.get());

KFilipek · 2025-10-08T18:26:06Z

examples/ctl/custom_ctl.c

+    int a;
+    int b;
+    int c;
+    int m; // modulus value, optional


typedef struct ctl_provider_t { int a, b, c, m; } ctl_provider_t;

KFilipek · 2025-10-08T18:34:27Z

examples/ctl/CMakeLists.txt

+    PRIVATE
+    ${
+    LIBHWLOC_LIBRARY_DIRS})
+target_link_libraries(${ EXAMPLE_NAME} PRIVATE ${LIBUMF_LIBRARIES} hwloc)


Is needed to keep those spaces in brackets?
I mean:
${hereEXAMPLE_NAME}

KFilipek · 2025-10-08T18:36:57Z

include/umf/memory_provider_ops.h

        void *provider, const void *ptr,
        umf_memory_property_id_t memory_property_id, void *property_value);

+    ///


In memory_pool.h you removed such line :)

KFilipek · 2025-10-08T18:39:25Z

src/ctl/ctl_defaults.c

+                                 umf_ctl_query_type_t queryType) {
+    (void)source;
+    if (strstr(extra_name, "{}") != NULL) {
+        LOG_ERR("%s, default setting do not support wildcard parameters {}",


do not -> does not?

KFilipek · 2025-10-08T18:46:38Z

test/common/fork_helpers.hpp

+#include "umf.h"
+#include <gtest/gtest.h>


different header groups

lplewa changed the title ~~Ctl split~~ The last CTL patch :) Sep 17, 2025

lplewa force-pushed the ctl_split branch 11 times, most recently from 629b379 to 2706166 Compare September 18, 2025 14:44

lplewa and others added 5 commits September 18, 2025 16:53

add defaults to providers

c12dcb1

add post-initialize function to pools and providers

8208bf9

Split between initialize and post-initialize function is necessary for properly handling CTL defaults.

Initial documentation

f602262

add new error to indicate unknown/unsuported path in ctl

1bdfdf0

add params override for disjointpool

68991d4

lplewa force-pushed the ctl_split branch from 2706166 to 9e465ec Compare September 18, 2025 15:04

lplewa marked this pull request as ready for review September 18, 2025 15:22

lplewa requested a review from a team as a code owner September 18, 2025 15:22

lukaszstolarczuk requested changes Sep 19, 2025

View reviewed changes

lplewa added 5 commits September 19, 2025 15:45

use fork for default tests

f8cc704

defaults changes global state, so using forks will isolate tests

rename name to path in ctl

55d4f0a

remove tests for changing pool name thru ctl

db6df68

use actual pool name instead of default name

eff4dbd

Documentation improvements

15e7433

lplewa force-pushed the ctl_split branch from 3b5e16b to 94517a8 Compare September 19, 2025 13:45

rename examples and mark ctl as experimental in those

decb1fd

lplewa force-pushed the ctl_split branch from 94517a8 to decb1fd Compare September 19, 2025 14:44

lukaszstolarczuk reviewed Oct 3, 2025

View reviewed changes

bratpiorka requested changes Oct 8, 2025

View reviewed changes

bratpiorka requested a review from Copilot October 8, 2025 08:37

Copilot AI reviewed Oct 8, 2025

View reviewed changes

KFilipek requested changes Oct 8, 2025

View reviewed changes


		UMF_CONF="umf.logger.output=stdout;umf.logger.level=0"

		CTL options available through environment variables are limited—you can only

		#include "umf.h"
		#include <gtest/gtest.h>

The last CTL patch :) #1524

Are you sure you want to change the base?

The last CTL patch :) #1524

Uh oh!

Conversation

lplewa commented Sep 17, 2025 • edited by lukaszstolarczuk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lplewa Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

lplewa commented Sep 17, 2025 •

edited by lukaszstolarczuk

Loading

lplewa Oct 6, 2025 •

edited

Loading