Summary: | [ksysguardd]: Internal buffer too small to read '/proc/cpuinfo' | ||
---|---|---|---|
Product: | [Unmaintained] ksysguard | Reporter: | Alessandro <alessandro.sturniolo> |
Component: | ksysguardd | Assignee: | KSysGuard Developers <ksysguard-bugs> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | ahiemstra, billfleming11, bugreporter11, bugs.kde.org.facelift226, dustin, jakob.kummerow, kdebugs.81do7, nate, rdieter, wadlax |
Priority: | NOR | ||
Version: | 5.17.4 | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
See Also: | https://bugs.kde.org/show_bug.cgi?id=419353 | ||
Latest Commit: | https://commits.kde.org/ksysguard/031bd7603e39dad609a597c64ff66fbec418d999 | Version Fixed In: | 5.18.3 |
Sentry Crash Report: | |||
Attachments: | attachment-5249-0.html |
Description
Alessandro
2017-09-09 11:18:57 UTC
The internal buffer only has 32 * 1024 bytes, assuming that the number of CPUs is limited to 32, and each entry is around 1K big. Now, with the ever increasing number of flags, the per-CPU entry is bigger, and should probably be raised to 1.5K. Newer x86_64 kernels, however, allow up to 4096 CPUs, so either the buffer needs to be much bigger, or we should interleaved /proc/cpuinfo reading and parsing. See ksysguard.git/ksysguardd/Linux/cpuinfo.c This affects me on Arch Linux. On my sytem `cat /proc/cpuinfo` lists 40 processors. The system has two Intel(R) Xeon(R) CPU E5-2630 v4 @ 2.20GHz with 10 cores each. *** Bug 410678 has been marked as a duplicate of this bug. *** *** Bug 414733 has been marked as a duplicate of this bug. *** This happens on Arch Linux as well with the latest KDE packages. No info is shown in the System Load Plasma widget and 'Internal buffer too small to read '/proc/cpuinfo' is shown in journalctl. This didn't used to happen on my Ryzen 1800x but is now happening on my Ryzen 3900x. The increase in the number of cores is probably making /proc/cpuinfo pretty big. I'm affected by this too. I have two data points to offer, via `cat /proc/cpuinfo | wc -c`: - on a 12C24T AMD machine: ~36K - on a 36C72T Intel machine: ~102K Extrapolating, I estimate that the biggest CPUs available today (Ryzen 3990, 64C128T) will need ~192K (but I don't have one of those available to test on). I'm surprised to see that this issue has been reported more than two years ago. Is there really no one who could commit a one-liner fix? Please just change the line at https://cgit.kde.org/ksysguard.git/tree/ksysguardd/Linux/cpuinfo.c#n43 from: #define CPUINFOBUFSIZE (32 * 1024) to: #define CPUINFOBUFSIZE (256 * 1024) I've created a pull request: https://phabricator.kde.org/D27362 Created attachment 125936 [details] attachment-5249-0.html I'm happy for someone to commit this (I'm the maintainer, but can't do it myself currently sorry) On Thu, Feb 13, 2020, 20:23 Jakob Kummerow <bugzilla_noreply@kde.org> wrote: > https://bugs.kde.org/show_bug.cgi?id=384515 > > --- Comment #7 from Jakob Kummerow <jakob.kummerow@gmail.com> --- > I've created a pull request: https://phabricator.kde.org/D27362 > > -- > You are receiving this mail because: > You are watching the assignee of the bug. Git commit 62df377051f4b266974de5751d5f9363030a6670 by Arjen Hiemstra. Committed on 17/02/2020 at 10:19. Pushed by ahiemstra into branch 'master'. Linux/cpuinfo.c: grow buffer size as needed for 12+ core CPUs Summary: Getting CPU information starts with reading /proc/cpuinfo into a buffer; if that buffer is too small, then no information will be returned at all (updateCpuInfo() will return -1, so initCpuInfo() will return early); the consequence is that ksysguardd/ksysguard/plasmaengineexplorer won't know about "system/cores" or "cpu/system/AverageClock"; that in turn will make the "System Load Viewer" plasmoid completely dysfunctional (it won't show *any* data) because it relies on "system/cores" being available. The buffer is currently 32KiB large, which is not enough on modern hardware (e.g. 12-core/24-thread systems exceed it). This patch lowers the initial size to 8KiB to be as memory-efficient as possible on low-end systems (dual/quadcore systems shouldn't ever need to grow it), and grows the buffer size as needed to accommodate arbitrarily many CPUs (tested on a 72-thread system, where the buffer grows four times, reaching 128KiB final size). When the buffer needs to grow, its size is doubled, so the overall cost of the growth scales linearly with the file size (on average, each byte is copied to a larger buffer at most once). Also, the buffer size is remembered as long as the process runs, so the cost of the growth is only incurred once on startup (or if additional CPUs come online, which is probably rare). I've verified locally that this patch fixes the issue: the System Log Viewer plasmoid shows data as expected afterwards. Reviewers: davidedmundson, ahiemstra Reviewed By: ahiemstra Subscribers: ahiemstra, cfeck, plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D27362 M +15 -8 ksysguardd/Linux/cpuinfo.c https://commits.kde.org/ksysguard/62df377051f4b266974de5751d5f9363030a6670 Git commit 7a103a45e3496b57ed76f99e89ea18cbabdba8c7 by Arjen Hiemstra, on behalf of Jakob Kummerow. Committed on 19/02/2020 at 10:49. Pushed by ahiemstra into branch 'master'. Linux/cpuinfo.c: grow buffer size as needed for 12+ core CPUs Summary: Getting CPU information starts with reading /proc/cpuinfo into a buffer; if that buffer is too small, then no information will be returned at all (updateCpuInfo() will return -1, so initCpuInfo() will return early); the consequence is that ksysguardd/ksysguard/plasmaengineexplorer won't know about "system/cores" or "cpu/system/AverageClock"; that in turn will make the "System Load Viewer" plasmoid completely dysfunctional (it won't show *any* data) because it relies on "system/cores" being available. The buffer is currently 32KiB large, which is not enough on modern hardware (e.g. 12-core/24-thread systems exceed it). This patch lowers the initial size to 8KiB to be as memory-efficient as possible on low-end systems (dual/quadcore systems shouldn't ever need to grow it), and grows the buffer size as needed to accommodate arbitrarily many CPUs (tested on a 72-thread system, where the buffer grows four times, reaching 128KiB final size). When the buffer needs to grow, its size is doubled, so the overall cost of the growth scales linearly with the file size (on average, each byte is copied to a larger buffer at most once). Also, the buffer size is remembered as long as the process runs, so the cost of the growth is only incurred once on startup (or if additional CPUs come online, which is probably rare). I've verified locally that this patch fixes the issue: the System Log Viewer plasmoid shows data as expected afterwards. Reviewers: davidedmundson, ahiemstra Reviewed By: ahiemstra Subscribers: ngraham, ahiemstra, cfeck, plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D27362 M +15 -8 ksysguardd/Linux/cpuinfo.c https://commits.kde.org/ksysguard/7a103a45e3496b57ed76f99e89ea18cbabdba8c7 Arjen, do you think this could this be cherry-picked to the stable branch? Doesn't look super high risk to me. (In reply to Nate Graham from comment #11) > Arjen, do you think this could this be cherry-picked to the stable branch? > Doesn't look super high risk to me. If it can be useful, I'm already using this patch, since it was released, on ksysguard 5.18 with no problems. Git commit 031bd7603e39dad609a597c64ff66fbec418d999 by Arjen Hiemstra, on behalf of Jakob Kummerow. Committed on 05/03/2020 at 10:04. Pushed by ahiemstra into branch 'Plasma/5.18'. Linux/cpuinfo.c: grow buffer size as needed for 12+ core CPUs Summary: Getting CPU information starts with reading /proc/cpuinfo into a buffer; if that buffer is too small, then no information will be returned at all (updateCpuInfo() will return -1, so initCpuInfo() will return early); the consequence is that ksysguardd/ksysguard/plasmaengineexplorer won't know about "system/cores" or "cpu/system/AverageClock"; that in turn will make the "System Load Viewer" plasmoid completely dysfunctional (it won't show *any* data) because it relies on "system/cores" being available. The buffer is currently 32KiB large, which is not enough on modern hardware (e.g. 12-core/24-thread systems exceed it). This patch lowers the initial size to 8KiB to be as memory-efficient as possible on low-end systems (dual/quadcore systems shouldn't ever need to grow it), and grows the buffer size as needed to accommodate arbitrarily many CPUs (tested on a 72-thread system, where the buffer grows four times, reaching 128KiB final size). When the buffer needs to grow, its size is doubled, so the overall cost of the growth scales linearly with the file size (on average, each byte is copied to a larger buffer at most once). Also, the buffer size is remembered as long as the process runs, so the cost of the growth is only incurred once on startup (or if additional CPUs come online, which is probably rare). I've verified locally that this patch fixes the issue: the System Log Viewer plasmoid shows data as expected afterwards. Reviewers: davidedmundson, ahiemstra Reviewed By: ahiemstra Subscribers: ngraham, ahiemstra, cfeck, plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D27362 M +15 -8 ksysguardd/Linux/cpuinfo.c https://commits.kde.org/ksysguard/031bd7603e39dad609a597c64ff66fbec418d999 (In reply to Nate Graham from comment #11) > Arjen, do you think this could this be cherry-picked to the stable branch? > Doesn't look super high risk to me. It does. Sorry it took me a while, but the commit has now been cherry-picked to 5.18 so should become part of 5.18.3. Thanks! |