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

[Pal] Remove STATIC_SLAB flag #95

Merged
merged 1 commit into from
Sep 28, 2021
Merged

[Pal] Remove STATIC_SLAB flag #95

merged 1 commit into from
Sep 28, 2021

Conversation

pwmarcz
Copy link
Contributor

@pwmarcz pwmarcz commented Sep 27, 2021

PAL doesn't work without STATIC_SLAB anymore.

(As discussed in #32).

How to test this PR?

Jenkins should be enough, there are no functional changes.


This change is Reviewable

Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @pwmarcz)


-- commits, line 4 at r1:
I don't think it's "anymore", it's like "never really worked". This is from the Chia-Che times :)


Pal/include/pal_defs.h, line 5 at r1 (raw file):

/* maximum length of URIs */
#define URI_MAX 4096

Now that we have only one macro in this file, can we just move this file somewhere else and remove this header file altogether?

But if you think it's orthogonal to this PR, I will resolve.


Pal/src/slab.c, line 30 at r1 (raw file):

static void* g_high = &g_mem_pool[POOL_SIZE];
#else
#define ALLOC_ALIGNMENT g_slab_alignment

Heh, looks like this macro was never used

Copy link
Contributor Author

@pwmarcz pwmarcz left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 52 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv)


-- commits, line 4 at r1:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I don't think it's "anymore", it's like "never really worked". This is from the Chia-Che times :)

OK. Changed.


Pal/include/pal_defs.h, line 5 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Now that we have only one macro in this file, can we just move this file somewhere else and remove this header file altogether?

But if you think it's orthogonal to this PR, I will resolve.

And URI_MAX probably belongs in pal.h anyway.

Removed. It looks like we included that file everywhere, but sed /pal_defs.h/d took care of things.


Pal/src/slab.c, line 30 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Heh, looks like this macro was never used

It was a parameter to slabmgr.h.

dimakuv
dimakuv previously approved these changes Sep 27, 2021
Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 52 of 52 files at r2, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

mkow
mkow previously approved these changes Sep 28, 2021
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 52 of 52 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

PAL never really worked without STATIC_SLAB.

Signed-off-by: Paweł Marczewski <pawel@invisiblethingslab.com>
@dimakuv dimakuv dismissed stale reviews from mkow and themself via b2e1aab September 28, 2021 12:55
Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@mkow mkow merged commit b2e1aab into master Sep 28, 2021
@mkow mkow deleted the pawel/static-slab branch September 28, 2021 14:25
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.

3 participants