After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 789037 - [PATCH] fix build with kernel-4.7
[PATCH] fix build with kernel-4.7
Status: RESOLVED FIXED
Product: gnome-applets
Classification: Other
Component: cpufreq
3.26.x
Other Linux
: Normal major
: ---
Assigned To: gnome-applets Maintainers
gnome-applets Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-10-16 00:08 UTC by Balló György
Modified: 2017-10-16 17:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix build with kernel-4.7 (996 bytes, patch)
2017-10-16 00:08 UTC, Balló György
none Details | Review
cpufreq: fix build with -lcpupower and kernel 4.7+ (4.06 KB, patch)
2017-10-16 11:17 UTC, Alberts Muktupāvels
committed Details | Review
configure output (19.00 KB, text/x-log)
2017-10-16 16:53 UTC, Balló György
  Details

Description Balló György 2017-10-16 00:08:11 UTC
Created attachment 361646 [details] [review]
fix build with kernel-4.7

I had to apply the attached patch from mate-applets,[1] otherwise the CPUFreq Applet fails to run with the following error:

** (gnome-panel:1361): WARNING **: Failed to load applet CPUFreqAppletFactory::CPUFreqApplet: /usr/lib/gnome-applets/libcpu-frequency-applet.so: undefined symbol: cpufreq_cpu_exists

The patch fixes the problem for me. I'm using the '--with-cpufreq-lib=cpupower' build option on Arch Linux with kernel version 4.13.

[1] https://github.com/mate-desktop/mate-applets/pull/194
Comment 1 Dmitry Shachnev 2017-10-16 07:47:07 UTC
Review of attachment 361646 [details] [review]:

::: cpufreq/src/cpufreq-monitor.c
@@ +81,3 @@
       if (!cpufreq_cpu_exists (monitor->cpu))
+#else
+      if (cpupower_is_cpu_online (monitor->cpu))

It seems a bit strange to me that this condition is not negated. Should it be !cpupower_is_cpu_online maybe? In any case, is it really equivalent to !cpufreq_cpu_exists?
Comment 2 Balló György 2017-10-16 08:16:01 UTC
I have no idea. I just copied the code from mate-applets, and it fixes the applet for me.
Comment 3 Alberts Muktupāvels 2017-10-16 08:23:47 UTC
Does not that break build with old cpufreq lib?

Anyway my idea is to remove option to switch libs and try to autodetect first cpupower and fallback to cpufreq, but...

I will need to search what cpufreq_cpu_exists and cpupower_is_cpu_online does and if they do same thing.
Comment 4 Alberts Muktupāvels 2017-10-16 09:23:03 UTC
1) https://git.gnome.org/browse/gnome-applets/tree/cpufreq/src/cpufreq-monitor.c#n78

Check whether it failed because cpu is not online.

2) https://github.com/torvalds/linux/blob/2dcd0af568b0cf583645c8a317dd12e344b1c72a/tools/power/cpupower/lib/cpufreq.h#L61

returns 0 if the specified CPU is present (it doesn't say  whether it is online!), and an error value if not.

3) https://github.com/torvalds/linux/blob/master/tools/power/cpupower/lib/cpupower.c#L39

"Detect whether a CPU is online
Returns: 1 -> if CPU is online
         0 -> if CPU is offline
         negative errno values in error case

---

So I would say that existing function is not correct, because cpufreq_cpu_exists does not tell if CPU is online or offline or comment is wrong.

That patch is wrong, because it will mark CPU as offline if that function returns error (negative errno values in error case) or 1 (if CPU is online). I doubt that we want that cpufreq applets thinks that cpu is offline when actually it is online.

That if should be:
if (cpupower_is_cpu_online (monitor->cpu) != 1)
or:
if (cpupower_is_cpu_online (monitor->cpu) < 1)
Comment 5 Alberts Muktupāvels 2017-10-16 11:17:00 UTC
Created attachment 361660 [details] [review]
cpufreq: fix build with -lcpupower and kernel 4.7+

This patch removes --with-cpufreq-lib configure option! Now
GNOME Applets will try to autodetect cpupower library and will
fallback to cpufreq if it is not available.

In kernel 4.7+ cpufreq_cpu_exists function was removed, replace
it with cpupower_is_cpu_online which seems more suitable than
old function at least according to comment above if statement.
Comment 6 Alberts Muktupāvels 2017-10-16 11:18:55 UTC
Please test above patch!

It should use -lcpupower if library is available and it should work with kernel 4.7+ and also with older version.

If -lcpupower is not available it will try to use old/deprecated/unmaintained -lcpufreq library.
Comment 7 Balló György 2017-10-16 14:59:04 UTC
I tested your patch, but it fails to build for me with the following error (cpupower 4.13 is available on the system):

cpufreq-selector-service.c: In function ‘get_selector_for_cpu’:
cpufreq-selector-service.c:260:30: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (service->selectors_max < cpu)
                              ^
  CCLD     cpufreq-selector
/usr/bin/ld: cannot find -lcpufreq
collect2: error: ld returned 1 exit status
Comment 8 Alberts Muktupāvels 2017-10-16 15:00:28 UTC
attach output from configure.
Comment 9 Balló György 2017-10-16 16:53:10 UTC
Created attachment 361688 [details]
configure output

Here is the log.
Comment 10 Alberts Muktupāvels 2017-10-16 16:55:54 UTC
Are you sure that you applied patch?

checking cpufreq.h usability... yes
checking cpufreq.h presence... yes
checking for cpufreq.h... yes
checking whether to enable ipv6... yes

It should include after cpufreq.h at least one more line:
checking for cpupower_is_cpu_online in -lcpupower... yes

or:
checking for cpupower_is_cpu_online in -lcpupower... no
checking for cpufreq_cpu_exists in -lcpupower... no
checking for cpufreq_cpu_exists in -lcpufreq... yes
Comment 11 Balló György 2017-10-16 17:21:08 UTC
Oh, I wasn't applied your patch properly.

Your patch works fine. Thank you very much!