-
Notifications
You must be signed in to change notification settings - Fork 605
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
Set /dev/shm size from /proc/meminfo #770
Conversation
c := cli.Command{ | ||
Name: "mount-shm", | ||
Usage: "Set up the /dev/shm mount required by the MPS daemon", | ||
Action: func(ctx *cli.Context) error { | ||
return mountShm(ctx, &opts) | ||
}, |
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.
Can we have this component directly use the plugins config
file and avoid introducing flags?
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 will remove the flags. I will work in the use of the config as part of #699 if we determine that this needs to be set.
One thing to note is that allowing this option to be set complicates the syncronisation between the various components considerably since this container would also need to be restarted on a config change.
|
||
// getShmSize returns the default shm size. | ||
// This reads /proc/meminfo to get the total memory to calculate this. | ||
func (o *options) getShmSize() (size string) { |
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.
func (o *options) getShmSize() (size string) { | |
func (o *options) getDefaultShmSize() (size string) { |
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.
Then maybe the comment needs to change?
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.
It doesn't get the default shm size. It also returns the size if set. Note that this assumes that we want to allow this to be set. I could remove the interactions with options
to simplify.
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.
Removed the addition of options
. I will add that once it becomes relevant.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
569bc89
to
3d4e2f2
Compare
80b9354
to
8e6713d
Compare
This change reads MemTotal from /proc/meminfo and sets the dev/shm size to half this value. This aligns with the default for creating tmpfs mounts. If reading the value fails, the previous default of 65536k is used. Signed-off-by: Evan Lezar <elezar@nvidia.com>
8e6713d
to
1f5120c
Compare
This change reads MemTotal from /proc/meminfo and sets
the dev/shm size to half this value. This aligns with the
default for creating tmpfs mounts.
If reading the value fails, the previous default of 65536k is used.