lib/ukstore: Introduce dynamic store API #939

michpappas · 2023-06-08T04:34:03Z

Prerequisite checklist

Read the contribution guidelines regarding submitting new changes to the project;
Tested your changes against relevant architectures and platforms;
Ran the checkpatch.uk on your commit series before opening this PR;
Updated relevant documentation.

Base target

Architecture(s): [e.g. x86_64 or N/A]
Platform(s): [e.g. kvm, xen or N/A]
Application(s): [e.g. app-python3 or N/A]

Additional configuration

Description of changes

This series adds dynamic entry support to uk_store. Libraries can dynamically register uk_store objects at runtime, each one of which is associated with one or more entries. Similarly to static entries, each dynamic entry is associated with a getter and / or a setter.

Consumers are notified for the lifetime of objects via events.

GitHub-Supersedes: #460
GitHub-Depends-On: #938

lib/ukalloc/include/uk/alloc_store.h

craciunoiuc

Some small comments from my side, it looks good 🙏

I hope you also modified the code to be in accordance with the new coding conventions 😅

lib/ukstore/README.md

craciunoiuc · 2023-06-08T09:15:40Z

I saw also some errors in checkpatch, if those are actually ok, add the Checkpatch-Ignore: ... in the message

craciunoiuc · 2023-06-08T09:17:28Z

In the PR text also add Closes for automatic closing when merged

https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests

craciunoiuc · 2023-06-08T12:34:27Z

When @skuenzer is happy I'll also add my tag and we can merge

lib/ukstore/include/uk/store.h

skuenzer · 2023-08-16T14:37:57Z

lib/ukstore/include/uk/store.h

+ * @param library_id owner library id as returned by uklibid
+ * @param object_id  object id
+ */
+void uk_store_release_object(__u16 library_id, __u64 object_id);


Would void uk_store_release_object(struct uk_store_object *obj); work as well? I remember we had a longer discussion about this but I don't remember the result.

From a look at the client side, I see no reason why we can't do that. Yet this needs to be changed with clear mind as it affects how we lock the refcounts. I will look at it tomorrow morning.

I agree, sounds good.

lib/ukstore/include/uk/store.h

michpappas · 2023-08-16T19:31:50Z

@skuenzer I addressed all review comments except from #939 (comment) which I will look at tomorrow morning. I marked the rest as resolved.

Signed-off-by: Michalis Pappas <michalis@unikraft.io>

Checkpatch-Ignore: LONG_LINE Signed-off-by: Michalis Pappas <michalis@unikraft.io>

michpappas · 2023-08-17T14:05:09Z

@skuenzer all changes are in now, including an updated README.md.

Remove cookie from static uk_store entries. This is in anticipation of dynamic entries that provide a cookie at the object level. Signed-off-by: Michalis Pappas <michalis@unikraft.io>

Introduce an id property to uk_store entries and replace name with id on lookup operations, to avoid costly string comparison. Signed-off-by: Michalis Pappas <michalis@unikraft.io>

…initions Update calls to UK_STORE_STATIC_ENTRIES to remove the deprecated cookie parameter. Signed-off-by: Michalis Pappas <michalis@unikraft.io>

Introduce header for definitions related to uk_store. Add entry IDs for stats tracked by uk_alloc, and update static entry definitions to pass entry IDs. Signed-off-by: Michalis Pappas <michalis@unikraft.io>

This commit adds dynamic entry support to uk_store. Libraries can dynamically register uk_store objects at runtime, each one of which is associated with one or more entries. Similarly to static entries, each dynamic entry is associated with a getter and / or a setter. Consumers are notified for the lifetime of objects via events. Checkpatch-Ignore: SPACING LONG_LINE GitHub-Closes: unikraft#539 Co-authored-by: Michalis Pappas <michalis@unikraft.io> Signed-off-by: Cezar Craciunoiu <cezar.craciunoiu@gmail.com> Signed-off-by: Michalis Pappas <michalis@unikraft.io>

Signed-off-by: Michalis Pappas <michalis@unikraft.io>

skuenzer · 2023-08-17T20:34:51Z

@michpappas : I just rebased this PR to latest staging and removed the commit "lib/ukstore: Bring function definitions into the private scope", as we discussed.

unikraft-bot · 2023-08-17T20:50:24Z

✅ Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA	commit	checkpatch
`22d81e6`	lib/ukstore: Remove cookie from static entries	✅
`0f5f7e5`	lib/ukstore: Lookup entries by id	✅
`abcb269`	lib/ukalloc: Remove deprecated cookie parameter from static entry definitions	✅
`3ef1dae`	lib/ukalloc: Add entry IDs and update static entry definitions	✅
`77c0aa5`	lib/ukstore: Introduce dynamic API	✅
`77c0aa5`	`Checkpatch-Ignore: SPACING LONG_LINE`	✅
`2e7d060`	lib/ukstore: Add README.md	✅

skuenzer

Thanks a lot for all your work @craciunoiuc and @michpappas

Reviewed-by: Simon Kuenzer simon@unikraft.io
Approved-by: Simon Kuenzer simon@unikraft.io

craciunoiuc

All good here. Thanks!

Reviewed-by: Cezar Craciunoiu cezar.craciunoiu@unikraft.io

Introduce an id property to uk_store entries and replace name with id on lookup operations, to avoid costly string comparison. Signed-off-by: Michalis Pappas <michalis@unikraft.io> Reviewed-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #939

…initions Update calls to UK_STORE_STATIC_ENTRIES to remove the deprecated cookie parameter. Signed-off-by: Michalis Pappas <michalis@unikraft.io> Reviewed-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #939

Introduce header for definitions related to uk_store. Add entry IDs for stats tracked by uk_alloc, and update static entry definitions to pass entry IDs. Signed-off-by: Michalis Pappas <michalis@unikraft.io> Reviewed-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #939

This commit adds dynamic entry support to uk_store. Libraries can dynamically register uk_store objects at runtime, each one of which is associated with one or more entries. Similarly to static entries, each dynamic entry is associated with a getter and / or a setter. Consumers are notified for the lifetime of objects via events. Checkpatch-Ignore: SPACING LONG_LINE GitHub-Closes: #539 Co-authored-by: Michalis Pappas <michalis@unikraft.io> Signed-off-by: Cezar Craciunoiu <cezar.craciunoiu@gmail.com> Signed-off-by: Michalis Pappas <michalis@unikraft.io> Reviewed-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #939

Signed-off-by: Michalis Pappas <michalis@unikraft.io> Reviewed-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #939

michpappas requested review from a team as code owners June 8, 2023 04:34

michpappas assigned skuenzer Jun 8, 2023

michpappas requested a review from craciunoiuc June 8, 2023 04:34

unikraft-bot added area/lib Internal Unikraft Microlibrary lang/c Issues or PRs to do with C/C++ lib/syscall_shim lib/ukalloc lib/ukdebug labels Jun 8, 2023

craciunoiuc reviewed Jun 8, 2023

View reviewed changes

lib/ukalloc/include/uk/alloc_store.h Outdated Show resolved Hide resolved

craciunoiuc requested changes Jun 8, 2023

View reviewed changes

lib/ukstore/README.md Outdated Show resolved Hide resolved

lib/ukstore/README.md Outdated Show resolved Hide resolved

lib/ukstore/README.md Outdated Show resolved Hide resolved

michpappas force-pushed the uk_store_dynamic_pr branch from 6445a3f to 0489936 Compare June 8, 2023 10:13

michpappas force-pushed the uk_store_dynamic_pr branch from 0489936 to 31db98c Compare June 13, 2023 04:34

This comment was marked as outdated.

Sign in to view

This was referenced Jun 13, 2023

lib/ukfalloc: Export stats via uk_store #942

Open

lib/uknetdev: Export stats via uk_store #943

Closed

craciunoiuc mentioned this pull request Jun 13, 2023

lib/ukstore: Introduce the ukstore library (dynamic) #460

Closed

razvand requested review from adinamariav and removed request for a team June 15, 2023 15:50

razvand added this to the v0.14.0 (Prometheus) milestone Jun 15, 2023

adinamariav approved these changes Jul 4, 2023

View reviewed changes

skuenzer reviewed Aug 16, 2023

View reviewed changes

michpappas force-pushed the uk_store_dynamic_pr branch from 93cb914 to 6d0c66b Compare August 16, 2023 19:28

michpappas added a commit to michpappas/unikraft that referenced this pull request Aug 16, 2023

lib/ukstore: Introduce dynamic API (PR unikraft#939)

f20838a

Signed-off-by: Michalis Pappas <michalis@unikraft.io>

michpappas added a commit to michpappas/unikraft that referenced this pull request Aug 17, 2023

lib/ukstore: Introduce dynamic API (PR unikraft#939)

bbd29ad

Checkpatch-Ignore: LONG_LINE Signed-off-by: Michalis Pappas <michalis@unikraft.io>

michpappas force-pushed the uk_store_dynamic_pr branch 3 times, most recently from 37a9f22 to 682c47b Compare August 17, 2023 14:01

michpappas force-pushed the uk_store_dynamic_pr branch 2 times, most recently from ad701a2 to e16a7fc Compare August 17, 2023 16:32

michpappas and others added 6 commits August 17, 2023 22:26

lib/ukstore: Remove cookie from static entries

22d81e6

Remove cookie from static uk_store entries. This is in anticipation of dynamic entries that provide a cookie at the object level. Signed-off-by: Michalis Pappas <michalis@unikraft.io>

lib/ukstore: Lookup entries by id

0f5f7e5

Introduce an id property to uk_store entries and replace name with id on lookup operations, to avoid costly string comparison. Signed-off-by: Michalis Pappas <michalis@unikraft.io>

lib/ukalloc: Remove deprecated cookie parameter from static entry def…

abcb269

…initions Update calls to UK_STORE_STATIC_ENTRIES to remove the deprecated cookie parameter. Signed-off-by: Michalis Pappas <michalis@unikraft.io>

lib/ukalloc: Add entry IDs and update static entry definitions

3ef1dae

Introduce header for definitions related to uk_store. Add entry IDs for stats tracked by uk_alloc, and update static entry definitions to pass entry IDs. Signed-off-by: Michalis Pappas <michalis@unikraft.io>

lib/ukstore: Add README.md

2e7d060

Signed-off-by: Michalis Pappas <michalis@unikraft.io>

skuenzer force-pushed the uk_store_dynamic_pr branch from e16a7fc to 2e7d060 Compare August 17, 2023 20:33

skuenzer approved these changes Aug 17, 2023

View reviewed changes

craciunoiuc approved these changes Aug 17, 2023

View reviewed changes

unikraft-bot closed this in 1de9465 Aug 17, 2023

unikraft-bot added the ci/merged Merged by CI label Aug 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/ukstore: Introduce dynamic store API #939

lib/ukstore: Introduce dynamic store API #939

michpappas commented Jun 8, 2023 •

edited by nderjung

Loading

craciunoiuc left a comment

craciunoiuc commented Jun 8, 2023

craciunoiuc commented Jun 8, 2023

craciunoiuc commented Jun 8, 2023

This comment was marked as outdated.

skuenzer Aug 16, 2023 •

edited

Loading

michpappas Aug 16, 2023

skuenzer Aug 16, 2023

michpappas commented Aug 16, 2023

michpappas commented Aug 17, 2023

skuenzer commented Aug 17, 2023 •

edited

Loading

unikraft-bot commented Aug 17, 2023

skuenzer left a comment

craciunoiuc left a comment

lib/ukstore: Introduce dynamic store API #939

lib/ukstore: Introduce dynamic store API #939

Conversation

michpappas commented Jun 8, 2023 • edited by nderjung Loading

Prerequisite checklist

Base target

Additional configuration

Description of changes

craciunoiuc left a comment

Choose a reason for hiding this comment

craciunoiuc commented Jun 8, 2023

craciunoiuc commented Jun 8, 2023

craciunoiuc commented Jun 8, 2023

This comment was marked as outdated.

skuenzer Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

michpappas Aug 16, 2023

Choose a reason for hiding this comment

skuenzer Aug 16, 2023

Choose a reason for hiding this comment

michpappas commented Aug 16, 2023

michpappas commented Aug 17, 2023

skuenzer commented Aug 17, 2023 • edited Loading

unikraft-bot commented Aug 17, 2023

skuenzer left a comment

Choose a reason for hiding this comment

craciunoiuc left a comment

Choose a reason for hiding this comment

michpappas commented Jun 8, 2023 •

edited by nderjung

Loading

skuenzer Aug 16, 2023 •

edited

Loading

skuenzer commented Aug 17, 2023 •

edited

Loading