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

Reduce the amount of memory allocated by KCM and avoid zeroing it when not necessary #7014

Closed
aplopez opened this issue Nov 3, 2023 · 1 comment
Assignees
Labels
Closed: Fixed Issue was closed as fixed. Work in Progress

Comments

@aplopez
Copy link
Contributor

aplopez commented Nov 3, 2023

Calls to sss_iobuf_init_empty() in KCM provide the initial size and the maximum capacity as parameters. In some cases the exact size of the required structure is passed as the initial size, but in other cases a generic value based on KCM_REPLY_MAX (currently 10 MiB) is provided.

  1. In these cases this initial value should be much lower, 1 or 2 KiB. Because this is an initial size, the structure can grow as needed until it reaches the maximum size of KCM_PACKET_MAX_SIZE.
  2. KCM_PACKET_MAX_SIZE and KCM_REPLY_MAX can possibly be merged into a single definition.
  3. Make sure sss_iobuf_init_empty() and related functions do not unnecessarily zero memory. Make sure other callers do not rely on sss_iobuf_init_empty() zeroing the memory.
@aplopez aplopez self-assigned this Nov 3, 2023
aplopez added a commit to aplopez/sssd that referenced this issue Nov 3, 2023
sss_iobuf_init_empty() and related functions zero the allocated memory
even though it is not needed. Most of the time, all the fields in the
structures will be set to non-zero values. In these cases zeroing the
is useless and we stop doing it.

Only in one case two pointers were being used with those NULL values,
so they are now being manually set to NULL.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 3, 2023
Some packages are being allocated to their maximum size, even though all
that memory is not required. When the amount of memory needed is not know,
We reduce the amount of memory allocated to the initial size defined by
the KCM_PACKET_INITIAL_SIZE macro.

The existing KCM_REPLY_MAX was replaced by KCM_PACKET_MAX_SIZE.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 3, 2023
sss_iobuf_init_empty() and related functions zero the allocated memory
even though it is not needed. Most of the time, all the fields in the
structures will be set to non-zero values. In these cases zeroing the
is useless and we stop doing it.

Only in two case, some pointers were being left unmodified, so they
are now being manually set to NULL.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 3, 2023
Some packages are being allocated to their maximum size, even though all
that memory is not required. When the amount of memory needed is not know,
We reduce the amount of memory allocated to the initial size defined by
the KCM_PACKET_INITIAL_SIZE macro.

The existing KCM_REPLY_MAX was replaced by KCM_PACKET_MAX_SIZE.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 7, 2023
sss_iobuf_init_empty() and related functions zero the allocated memory
even though it is not needed. Most of the time, all the fields in the
structures will be set to non-zero values. In these cases zeroing the
is useless and we stop doing it.

Only in two caseis, some pointers were being left unmodified, so they
are now being manually set to NULL.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 7, 2023
Some packages are being allocated to their maximum size, even though all
that memory is not required. When the amount of memory needed is not know,
We reduce the amount of memory allocated to the initial size defined by
the KCM_PACKET_INITIAL_SIZE macro.

The existing KCM_REPLY_MAX was replaced by KCM_PACKET_MAX_SIZE.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 13, 2023
sss_iobuf_init_empty() and related functions zero the allocated memory
even though it is not needed. Most of the time, all the fields in the
structures will be set to non-zero values. In these cases zeroing the
is useless and we stop doing it.

Only in two cases, some pointers were being left unmodified, so they
are now being manually set to NULL.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 13, 2023
Some packages are being allocated to their maximum size, even though all
that memory is not required. When the amount of memory needed is not know,
We reduce the amount of memory allocated to the initial size defined by
the KCM_PACKET_INITIAL_SIZE macro.

The existing KCM_REPLY_MAX was replaced by KCM_PACKET_MAX_SIZE.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 13, 2023
sss_iobuf_init_empty() and related functions zero the allocated memory
even though it is not needed. Most of the time, all the fields in the
structures will be set to non-zero values. In these cases zeroing the
is useless and we stop doing it.

Only in two cases, some pointers were being left unmodified, so they
are now being manually set to NULL.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 13, 2023
Some packages are being allocated to their maximum size, even though all
that memory is not required. When the amount of memory needed is not know,
We reduce the amount of memory allocated to the initial size defined by
the KCM_PACKET_INITIAL_SIZE macro.

The existing KCM_REPLY_MAX was replaced by KCM_PACKET_MAX_SIZE.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 13, 2023
sss_iobuf_init_empty() and related functions zero the allocated memory
even though it is not needed. Most of the time, all the fields in the
structures will be set to non-zero values. In these cases zeroing the
is useless and we stop doing it.

Only in two cases, some pointers were being left unmodified, so they
are now being manually set to NULL.

Resolves: SSSD#7014
aplopez added a commit to aplopez/sssd that referenced this issue Nov 13, 2023
Some packages are being allocated to their maximum size, even though all
that memory is not required. When the amount of memory needed is not know,
We reduce the amount of memory allocated to the initial size defined by
the KCM_PACKET_INITIAL_SIZE macro.

The existing KCM_REPLY_MAX was replaced by KCM_PACKET_MAX_SIZE.

Resolves: SSSD#7014
pbrezina pushed a commit that referenced this issue Nov 28, 2023
Some packages are being allocated to their maximum size, even though all
that memory is not required. When the amount of memory needed is not know,
We reduce the amount of memory allocated to the initial size defined by
the KCM_PACKET_INITIAL_SIZE macro.

The existing KCM_REPLY_MAX was replaced by KCM_PACKET_MAX_SIZE.

Resolves: #7014

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Tomáš Halman <thalman@redhat.com>
pbrezina pushed a commit that referenced this issue Nov 28, 2023
sss_iobuf_init_empty() and related functions zero the allocated memory
even though it is not needed. Most of the time, all the fields in the
structures will be set to non-zero values. In these cases zeroing the
is useless and we stop doing it.

Only in two cases, some pointers were being left unmodified, so they
are now being manually set to NULL.

Resolves: #7014

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Tomáš Halman <thalman@redhat.com>
(cherry picked from commit edb63cd)
pbrezina pushed a commit that referenced this issue Nov 28, 2023
Some packages are being allocated to their maximum size, even though all
that memory is not required. When the amount of memory needed is not know,
We reduce the amount of memory allocated to the initial size defined by
the KCM_PACKET_INITIAL_SIZE macro.

The existing KCM_REPLY_MAX was replaced by KCM_PACKET_MAX_SIZE.

Resolves: #7014

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Tomáš Halman <thalman@redhat.com>
(cherry picked from commit fe6c35a)
@pbrezina
Copy link
Member

Pushed PR: #7022

  • master
    • b4f9f63 - KCM: Do not zero memory when not need.
    • fe6c35a - KCM: Reduce the amount of memory allocated for the packages
    • edb63cd - KCM: sss_iobuf_init_empty() shall not zero memory
    • 2eb67af - KCM: When freeing the client, check that it is not NULL.
    • 1269205 - KCM: Remove unused cc_be_type from struct kcm_ccdb
    • 3cba6d1 - KCM: Fixed a wrong check
    • c73b7eb - KCM: Replace a hard-coded constant by a macro
  • sssd-2-9
    • 60fde9d - KCM: Do not zero memory when not need.
    • 78d0a97 - KCM: Reduce the amount of memory allocated for the packages
    • a5c96e2 - KCM: sss_iobuf_init_empty() shall not zero memory
    • 3e740a2 - KCM: When freeing the client, check that it is not NULL.
    • 14e7d7c - KCM: Remove unused cc_be_type from struct kcm_ccdb
    • 855d046 - KCM: Fixed a wrong check
    • 8c83234 - KCM: Replace a hard-coded constant by a macro

@pbrezina pbrezina added the Closed: Fixed Issue was closed as fixed. label Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed: Fixed Issue was closed as fixed. Work in Progress
Projects
None yet
3 participants