Skip to content

Conversation

@EricccTaiwan
Copy link
Collaborator

@EricccTaiwan EricccTaiwan commented May 28, 2025

Here are the changes I made:

  • Handle allocation failure.
  • Fix incorrect NULL check.
  • Unlock std_lock after allocation.
  • Switch to new mem block.
  • [RFC] Recompute padding on allocation $\to$ should this align with *scx_static_alloc(size_t bytes, size_t alignment) ?

Edit:

  • Fix tab usage.

@EricccTaiwan EricccTaiwan changed the title Rusty lib fix scx_rusty: Rusty lib fix May 28, 2025
@etsal etsal self-requested a review May 28, 2025 17:21
@etsal
Copy link
Contributor

etsal commented May 28, 2025

Yes, the arena code in rusty has been copied off of the main library, so this is essentially a backport. This includes recomputing the padding. Do you mind if I rename the pull request as "scx_rusty: arena library backports" for clarity?

@EricccTaiwan
Copy link
Collaborator Author

Yes, the arena code in rusty has been copied off of the main library, so this is essentially a backport. This includes recomputing the padding. Do you mind if I rename the pull request as "scx_rusty: arena library backports" for clarity?

Yes, renaming is clearer.
Since void __arena *sdt_static_alloc(size_t bytes) doesn’t take an alignment argument, should we rewrite it so that it recomputes padding the same way as scx_static_alloc(size_t bytes, size_t alignment)?

@EricccTaiwan EricccTaiwan changed the title scx_rusty: Rusty lib fix scx_rusty: arena library backports May 28, 2025
@etsal
Copy link
Contributor

etsal commented May 28, 2025

Oh I guess we had not added the padding feature to the allocator when we forked off the code, in that cases the padding is always 0 so it is not necessary.

@EricccTaiwan
Copy link
Collaborator Author

Oh I guess we had not added the padding feature to the allocator when we forked off the code, in that cases the padding is always 0 so it is not necessary.

OK, I've cleaned up the commit message, PTAL thanks!

EricccTaiwan and others added 5 commits May 30, 2025 02:22
Add a check for failed allocation when calling sdt_alloc().

Ref: 9649028

Co-authored-by: Po-Ying Chiu <charlie910417@gmail.com>
Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
Check `memory` instead of `scx_static.memory` after arena allocation.

Ref: 99b2c79

Co-authored-by: Po-Ying Chiu <charlie910417@gmail.com>
Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
Ref: ece44e4

Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
Add a missing `bpf_spin_unlock(&sdt_lock)` before returning
the allocated pointer to ensure the lock is always released
on the normal path.

Co-authored-by: Po-Ying Chiu <charlie910417@gmail.com>
Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
When the static allocator runs out of space it now allocates a fresh
arena block, resets the offset to zero.

Ref: 8d8a85d

Co-authored-by: Po-Ying Chiu <charlie910417@gmail.com>
Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
@etsal
Copy link
Contributor

etsal commented May 29, 2025

Thank you! I'll take another look.

@etsal etsal added this pull request to the merge queue May 29, 2025
Merged via the queue into sched-ext:main with commit 2433d96 May 29, 2025
16 checks passed
@EricccTaiwan EricccTaiwan deleted the rusty-lib-fix branch May 29, 2025 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants