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
[PATCH v5] linux-gen: ishm: implement huge page cache #685
Conversation
9d832ad
to
68e555b
Compare
this looks like ok, test case will be good to have. |
@MatiasElo Can you confirm this PR's affect on Bug 3774? |
Yes it does. |
Are the cached fds closed properly in terminate? |
@MatiasElo right, the file descriptors are not properly closed, a terminate function should be implemented. |
68e555b
to
0765cdb
Compare
With this patch, ODP will pre-allocate several huge pages at init time. When memory is to be mapped into a huge page, one that was pre-allocated will be used, if available, this way ODP won't have to trap into the kernel to allocate huge pages. The idea with this implementation is to trick ishm into thinking that a file descriptor where to map the memory was provided, this way it it won't try to allocate one itself. This file descriptor is one of those previously allocated at init time. When the system is done with this file descriptor, instead of closing it, it is put back into the list of available huge pages, ready to be reused. A collateral effect of this patch is that memory is not zeroed out when it is reused. WARNING: This patch will not work when using process mode threads. For several reasons, this may not work when using ODP_ISHM_SINGLE_VA either, so when this flag is set, the list of pre-allocated files is not used. By default ODP will not reserve any huge pages, to tell ODP to do that, update the ODP configuration file with something like this: shm: { num_cached_hp = 32 } Example usage: $ echo odp.config odp_implementation = "linux-generic" config_file_version = "0.0.1" shm: { num_cached_hp = 32 } $ ODP_CONFIG_FILE=odp.conf ./test/validation/api/shmem/shmem_main This patch solves bug #3774: https://bugs.linaro.org/show_bug.cgi?id=3774 Signed-off-by: Josep Puigdemont <josep.puigdemont@linaro.org>
0765cdb
to
3a98364
Compare
config/odp-linux-generic.conf
Outdated
@@ -18,6 +18,17 @@ | |||
odp_implementation = "linux-generic" | |||
config_file_version = "0.0.1" | |||
|
|||
# Internal shared memory allocator | |||
shm: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see something like:
# Shared memory options
shm: {
# Number of cached default size huge pages. These pages are allocated
# during odp_init_global() and freed back to the kernel in
# odp_term_global(). A value of zero means no pages are cached.
#
# When using process mode threads, this value should be set to 0 because
# the current implementation won't work properly otherwise.
num_cached_hp = 0
}
return fd; | ||
} | ||
|
||
static void hp_init(void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When thinking about the configuration option documentation I noticed it could be useful that hp_init() would cause odp_init_global() to fail if all requested pages are not successfully reserved. This way a user can be sure that a selected number of pages is available at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to provide a "best effort" cache, so to speak, but let's discuss during the meeting today how this should behave.
Signed-off-by: Josep Puigdemont <josep.puigdemont@linaro.org>
3a98364
to
6b0b24a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed-and-tested-by: Matias Elo matias.elo@nokia.com
For my own convenience, I've left the code to reserve huge pages on a "best effort" basis. However I think the team could perhaps discuss @MatiasElo suggestion during connect, and decide whether ODP should behave differently before an official release is done and we have to live with it forever. I also added the comment that negative numbers are reserved for the future. I thought that maybe a "-1" could mean that ODP should reserve all huge pages available to the system, for instance. |
Merged. |
With this patch, ODP will pre-allocate several huge pages at init
time. When memory is to be mapped into a huge page, one that was
pre-allocated will be used, if available, this way ODP won't have to
trap into the kernel to allocate huge pages.
The idea with this implementation is to trick ishm into thinking that
a file descriptor where to map the memory was provided, this way it
it won't try to allocate one itself. This file descriptor is one of
those previously allocated at init time. When the system is done with
this file descriptor, instead of closing it, it is put back into the
list of available huge pages, ready to be reused.
A collateral effect of this patch is that memory is not zeroed out
when it is reused.
WARNING: This patch will not work when using process mode threads.
For several reasons, this may not work when using ODP_ISHM_SINGLE_VA
either, so for this case the list of pre-allocated files is not used.
This patch should mitigate, if not solve, bug #3774:
https://bugs.linaro.org/show_bug.cgi?id=3774
To pre-allocate huge pages, define the environment variable
ODP_HP_CACHE, and possibly set it to the number of huge pages that
should be pre-allocated, setting it to -1 will reserve up to 32 huge
pages, which is currently a hard-coded limit.
example usage:
ODP_HP_CACHE=-1 ./test/validation/api/shmem/shmem_main
Signed-off-by: Josep Puigdemont josep.puigdemont@linaro.org