CTL: Add a CTL sources to the UMF #913

KFilipek · 2024-11-19T13:03:46Z

Description

This commit introduces sources and compilation of the CTL.

Checklists

TODO:

Move CTL sources
Make code compiled and symbols are exposed
Run tests

Other:

Code compiles without errors locally
All tests pass locally
CI workflows execute properly

CI workflows, not executed per PR (e.g. Nightly), execute properly
New tests added, especially if they will fail without my changes
Added/extended example(s) to cover this functionality
Extended the README/documentation
All newly added source files have a license
All newly added source files are referenced in CMake files
Logger (with debug/info/... messages) is used

src/ctl/alloc.h

src/ctl/os.h

src/ctl/queue.h

src/ctl/ctl.h

src/CMakeLists.txt

src/ctl/ctl.c

src/ctl/ctl.h

src/CMakeLists.txt

src/ctl/ctl.c

lukaszstolarczuk · 2024-11-19T13:56:09Z

src/ctl/ctl.c

+ */
+int ctl_load_config_from_string(struct ctl *ctl, void *ctx,
+                                const char *cfg_string) {
+    // LOG(3, "ctl %p ctx %p cfg_string \"%s\"", ctl, ctx, cfg_string);


LOG could be replaced with our LOG_* macros

Temporarily deleted

I'll leave this issue open, for LOGs to come back, when ready

bump, pls add LOGs

It's not a scope of this PR.

a few places with the old LOG( were removed, just please bring them back using our logger

src/ctl/ctl.h

src/ctl/alloc.c

cmake/helpers.cmake

src/ctl/ctl.h

src/ctl/ctl.c

lukaszstolarczuk · 2024-12-16T10:11:51Z

now all changes are in a single commit, at best the original code should be in the first commit and all our changes in a sep. one - this would be easier to distingush our changes, e.g. in case of any issues found

Thanks for introducing a sep. commit, but it still require some cleanup. At least the description of the 3rd commit should be rather in the first commit:

KFilipek · 2024-12-16T13:52:18Z

now all changes are in a single commit, at best the original code should be in the first commit and all our changes in a sep. one - this would be easier to distingush our changes, e.g. in case of any issues found

Thanks for introducing a sep. commit, but it still require some cleanup. At least the description of the 3rd commit should be rather in the first commit:

I've changed commits descriptions.

test/ctl/test.cpp

src/ctl/ctl.c

This commit introduces sources of the CTL. The CTL sources are copied from: https://github.com/pmem/pmdk/tree/master/src/common

Signed-off-by: Krzysztof Filipek <[email protected]>

lukaszstolarczuk

2 nits, 1 issue, beside it looks good

lukaszstolarczuk · 2024-12-19T09:59:05Z

src/ctl/ctl.c

+
+// This file was originally under following license:
+// SPDX-License-Identifier: BSD-3-Clause
+/* Copyright 2016-2024, Intel Corporation */


nit: do we need the second copyright? it's same company, it's even the same dates. If really needed we can merge them e.g. as in here: https://github.com/oneapi-src/unified-memory-framework/blob/main/src/uthash/uthash.h#L3

I will create an issue for it

lukaszstolarczuk · 2024-12-19T09:59:37Z

src/ctl/ctl.c

+ *
+ */
+
+// This file was originally under following license:


nit: why mixing 2 comment methods? we could stick to /**/

I will create an issue for it

KFilipek self-assigned this Nov 19, 2024

KFilipek added the enhancement New feature or request label Nov 19, 2024

lplewa requested changes Nov 19, 2024

View reviewed changes

src/ctl/alloc.h Outdated Show resolved Hide resolved

src/ctl/os.h Outdated Show resolved Hide resolved

src/ctl/queue.h Outdated Show resolved Hide resolved

lukaszstolarczuk reviewed Nov 19, 2024

View reviewed changes

KFilipek force-pushed the ctl-base branch 8 times, most recently from 5c52d3a to cdd3d46 Compare November 22, 2024 10:30

pbalcer reviewed Nov 22, 2024

View reviewed changes

src/ctl/alloc.c Outdated Show resolved Hide resolved

KFilipek force-pushed the ctl-base branch 15 times, most recently from 4f30764 to 0f8383e Compare November 29, 2024 08:32

KFilipek force-pushed the ctl-base branch 2 times, most recently from 0d68c68 to 44ec630 Compare December 3, 2024 10:12

bratpiorka marked this pull request as ready for review December 13, 2024 10:36

PatKamin reviewed Dec 13, 2024

View reviewed changes

cmake/helpers.cmake Outdated Show resolved Hide resolved

PatKamin reviewed Dec 13, 2024

View reviewed changes

src/ctl/ctl.h Outdated Show resolved Hide resolved

src/ctl/ctl.c Outdated Show resolved Hide resolved

src/ctl/ctl.c Show resolved Hide resolved

src/ctl/ctl.c Show resolved Hide resolved

KFilipek force-pushed the ctl-base branch from 05db80c to b320cf7 Compare December 16, 2024 13:52

KFilipek force-pushed the ctl-base branch 3 times, most recently from 19c3c68 to 8de1b69 Compare December 17, 2024 14:21

KFilipek requested review from lplewa, bratpiorka, lukaszstolarczuk and PatKamin December 17, 2024 14:25

PatKamin approved these changes Dec 17, 2024

View reviewed changes

bratpiorka approved these changes Dec 17, 2024

View reviewed changes

test/ctl/test.cpp Outdated Show resolved Hide resolved

bratpiorka reviewed Dec 17, 2024

View reviewed changes

test/ctl/test.cpp Outdated Show resolved Hide resolved

KFilipek force-pushed the ctl-base branch 3 times, most recently from 80da367 to 12dbce6 Compare December 17, 2024 15:54

lplewa reviewed Dec 17, 2024

View reviewed changes

src/ctl/ctl.c Show resolved Hide resolved

src/ctl/ctl.c Show resolved Hide resolved

src/ctl/ctl.c Outdated Show resolved Hide resolved

src/ctl/ctl.c Show resolved Hide resolved

src/ctl/ctl.c Outdated Show resolved Hide resolved

src/ctl/ctl.c Show resolved Hide resolved

KFilipek force-pushed the ctl-base branch from 12dbce6 to b0661f2 Compare December 17, 2024 18:44

KFilipek added 2 commits December 18, 2024 15:28

Move PMDK CTL sources

7f005d9

This commit introduces sources of the CTL. The CTL sources are copied from: https://github.com/pmem/pmdk/tree/master/src/common

[CMake] Disable pedantic mode and disable cast qualifier warning

5804207

KFilipek force-pushed the ctl-base branch from b0661f2 to 4138123 Compare December 18, 2024 14:45

CTL: Add a CTL functionality to the UMF

abb24e7

Signed-off-by: Krzysztof Filipek <[email protected]>

KFilipek force-pushed the ctl-base branch from 4138123 to abb24e7 Compare December 18, 2024 14:49

lplewa approved these changes Dec 18, 2024

View reviewed changes

lukaszstolarczuk reviewed Dec 19, 2024

View reviewed changes

KFilipek merged commit 2e3325f into oneapi-src:main Dec 19, 2024
77 checks passed

KFilipek mentioned this pull request Jan 13, 2025

CTL API proposal #1036

Open

CTL: Add a CTL sources to the UMF #913

CTL: Add a CTL sources to the UMF #913

Uh oh!

Conversation

KFilipek commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklists

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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukaszstolarczuk commented Dec 16, 2024

Uh oh!

KFilipek commented Dec 16, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukaszstolarczuk left a comment

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!

Uh oh!

Uh oh!

KFilipek commented Nov 19, 2024 •

edited

Loading