Skip to content

Fix comment in mx_proc #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix comment in mx_proc #117

wants to merge 1 commit into from

Conversation

pmenzel
Copy link
Contributor

@pmenzel pmenzel commented Dec 17, 2021

No description provided.

> pid + " (" comm + ") " + state + " " + pid + " \0"
> 10 + 2 + 15 + 2 + 1 + 1 + 10 + 2 = 43

Found-by: Donald
@pmenzel pmenzel mentioned this pull request Dec 17, 2021
@donald
Copy link
Contributor

donald commented Dec 17, 2021

Maybe I've got my 128 byte assumption from 1. However, it looks like 64 would be enough, because the caller for /proc/pid/stat disables escaping 2.

Instead of a comm value, which is limited to 15 characters, we might, in the case of a workqueue task, get a longer string, which is generated on the fly.

Linux documentation seems to be missleading 3 and we should consider to send a doc patch for that.

Copy link
Contributor

@donald donald left a comment

Choose a reason for hiding this comment

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

64 is not not enough for the buffer if we currently have nothing lower than 64 as the lower bound of the comm field alone. See my other comment. Sorry, it was me who suggested to shrink the array. Maybe a lower bound can be found by analysing the Linux source code further. Anyway, we should keep on the safe side with the buffer size. If the size was changed in the past by Linux developers (by adding workqueue names) it might change again. The bytes below the user stack pointer are cache hot anyway, because they are used when we call into glibc. Not that this would matter at all, its just an interesting thought. So we don't really care for a hundred bytes wasted. We should start worrying when we waste bytes in der order of page size. So maybe leave it at 128 but improve the comment.

Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants