Bug 384515 - [ksysguardd]: Internal buffer too small to read '/proc/cpuinfo'
Summary: [ksysguardd]: Internal buffer too small to read '/proc/cpuinfo'
Status: RESOLVED FIXED
Alias: None
Product: ksysguard
Classification: Applications
Component: ksysguardd (show other bugs)
Version: 5.17.4
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: KSysGuard Developers
URL:
Keywords:
: 410678 414733 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-09-09 11:18 UTC by Alessandro
Modified: 2020-03-29 04:28 UTC (History)
10 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.18.3


Attachments
attachment-5249-0.html (1.01 KB, text/html)
2020-02-13 11:28 UTC, John Tapsell
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alessandro 2017-09-09 11:18:57 UTC
The KDE widget don't show anything and on the system log, is present this message:

ksysguardd[4552]: Internal buffer too small to read '/proc/cpuinfo'

My workstation is a dual Opteron with 32 core, so the /proc/cpuinfo size is about 37kb
Comment 1 Christoph Feck 2017-09-15 23:30:05 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
Comment 2 bugreporter11 2018-04-05 19:31:40 UTC
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.
Comment 3 Christoph Feck 2019-08-07 08:47:30 UTC
*** Bug 410678 has been marked as a duplicate of this bug. ***
Comment 4 Christoph Feck 2020-01-01 19:50:42 UTC
*** Bug 414733 has been marked as a duplicate of this bug. ***
Comment 5 Benjamin Xiao 2020-02-02 19:47:30 UTC
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.
Comment 6 Jakob Kummerow 2020-02-13 11:03:41 UTC
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)
Comment 7 Jakob Kummerow 2020-02-13 11:23:32 UTC
I've created a pull request: https://phabricator.kde.org/D27362
Comment 8 John Tapsell 2020-02-13 11:28:51 UTC
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.
Comment 9 Arjen Hiemstra 2020-02-17 10:20:00 UTC
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
Comment 10 Arjen Hiemstra 2020-02-19 10:50:14 UTC
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
Comment 11 Nate Graham 2020-02-23 16:22:00 UTC
Arjen, do you think this could this be cherry-picked to the stable branch? Doesn't look super high risk to me.
Comment 12 Alessandro 2020-02-23 18:12:30 UTC
(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.
Comment 13 Arjen Hiemstra 2020-03-05 10:06:29 UTC
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
Comment 14 Arjen Hiemstra 2020-03-05 10:07:23 UTC
(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.
Comment 15 Nate Graham 2020-03-05 14:16:48 UTC
Thanks!