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.

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

lib/posix-process: Add option to context switch away from clone caller #1303

Conversation

mschlumpp
Copy link
Member

Some applications are expecting to run on a cooperative scheduler and will naively assume that the new thread will do some progress even if the clone caller does not context-switch by blocking/yielding.

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): N/A
  • Platform(s): N/A
  • Application(s): N/A

Additional configuration

  • CONFIG_LIBPOSIX_PROCESS_CLONE_PREFER_CHILD=y

Description of changes

@mschlumpp mschlumpp requested a review from a team as a code owner February 1, 2024 13:57
@github-actions github-actions bot added area/lib Internal Unikraft Microlibrary lang/c Issues or PRs to do with C/C++ lib/posix-process Information about system parameters labels Feb 1, 2024
@mschlumpp
Copy link
Member Author

Note: it is undecided whether we want to prioritize the parent or the children by default. (cc @michpappas)

@razvand razvand requested review from eduardvintila and Mihnea0Firoiu and removed request for a team February 2, 2024 08:16
@razvand razvand added this to the v0.16.2 (Telesto) milestone Feb 2, 2024
Copy link
Member

@eduardvintila eduardvintila left a comment

Choose a reason for hiding this comment

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

Nice addition, looks good to me.

Reviewed-by: Eduard Vintilă eduard.vintila47@gmail.com

Copy link
Member

@Mihnea0Firoiu Mihnea0Firoiu left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Reviewed-by: Mihnea Firoiu mihneafiroiu0@gmail.com

Copy link
Member

@michpappas michpappas left a comment

Choose a reason for hiding this comment

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

@mschlumpp just one minor comment below. Regarding ordering: As discussed offline, there isn't a good default in our case so let's leave it as is.

lib/posix-process/Config.uk Outdated Show resolved Hide resolved
@mschlumpp mschlumpp force-pushed the mschlumpp/feature/clone-deschedule branch from aa15b4c to a92bfff Compare February 7, 2024 08:08
@mschlumpp mschlumpp force-pushed the mschlumpp/feature/clone-deschedule branch from a92bfff to 4def7c7 Compare February 7, 2024 10:08
Some applications are expecting to run on a cooperative scheduler and will
naively assume that the new thread will do some progress even if the clone
caller does not context-switch by blocking/yielding.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
@mschlumpp mschlumpp force-pushed the mschlumpp/feature/clone-deschedule branch from 4def7c7 to 23683db Compare February 7, 2024 10:16
@michpappas
Copy link
Member

Approved-by: Michalis Pappas michalis@unikraft.io

@michpappas michpappas added the merge Label to trigger merge action label Feb 7, 2024
@unikraft-bot unikraft-bot changed the base branch from staging to staging-1303 February 7, 2024 10:24
@unikraft-bot unikraft-bot merged commit 8165799 into unikraft:staging-1303 Feb 7, 2024
13 checks passed
unikraft-bot pushed a commit that referenced this pull request Feb 7, 2024
Some applications are expecting to run on a cooperative scheduler and will
naively assume that the new thread will do some progress even if the clone
caller does not context-switch by blocking/yielding.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Approved-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Mihnea Firoiu <mihneafiroiu0@gmail.com>
GitHub-Closes: #1303
@unikraft-bot unikraft-bot added ci/merged Merged by CI and removed merge Label to trigger merge action labels Feb 7, 2024
@mschlumpp mschlumpp deleted the mschlumpp/feature/clone-deschedule branch February 7, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/lib Internal Unikraft Microlibrary ci/merged Merged by CI lang/c Issues or PRs to do with C/C++ lib/posix-process Information about system parameters
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants