diff mbox series

[v2] cpupower: avoid multiple definition with gcc -fno-common

Message ID 20200224202029.877025-1-floppym@gentoo.org (mailing list archive)
State Superseded, archived
Headers show
Series [v2] cpupower: avoid multiple definition with gcc -fno-common | expand

Commit Message

Mike Gilbert Feb. 24, 2020, 8:20 p.m. UTC
The -fno-common option will be enabled by default in GCC 10.

Bug: https://bugs.gentoo.org/707462
Signed-off-by: Mike Gilbert <floppym@gentoo.org>
---

v2: Made start_time static instead of extern.

 tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c  | 2 +-
 tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c    | 2 +-
 tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c | 2 ++
 tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h | 2 +-
 4 files changed, 5 insertions(+), 3 deletions(-)

Comments

Shuah Feb. 24, 2020, 10:26 p.m. UTC | #1
Hi Mike,

On 2/24/20 1:20 PM, Mike Gilbert wrote:
> The -fno-common option will be enabled by default in GCC 10.

Great. It landed in my inbox now.
Please include the message you are seeing in the change log.

> 
> Bug: https://bugs.gentoo.org/707462
> Signed-off-by: Mike Gilbert <floppym@gentoo.org>
> --- >> v2: Made start_time static instead of extern.
> 
>   tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c  | 2 +-
>   tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c    | 2 +-
>   tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c | 2 ++
>   tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h | 2 +-
>   4 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
> index 33dc34db4f3c..20f46348271b 100644
> --- a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
> +++ b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
> @@ -82,7 +82,7 @@ static struct pci_access *pci_acc;
>   static struct pci_dev *amd_fam14h_pci_dev;
>   static int nbp1_entered;
>   
> -struct timespec start_time;
> +static struct timespec start_time;
>   static unsigned long long timediff;

Does it make sense to move start_time and timediff defines to
cpupower-monitor.c and adding externs for them in
cpupower-monitor.h?

>   
>   #ifdef DEBUG
> diff --git a/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c b/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
> index 3c4cee160b0e..a65f7d011513 100644
> --- a/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
> +++ b/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
> @@ -19,7 +19,7 @@ struct cpuidle_monitor cpuidle_sysfs_monitor;
>   
>   static unsigned long long **previous_count;
>   static unsigned long long **current_count;
> -struct timespec start_time;
> +static struct timespec start_time;
>   static unsigned long long timediff;
>   
>   static int cpuidle_get_count_percent(unsigned int id, double *percent,
> diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> index 6d44fec55ad5..7c77045fef52 100644
> --- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> +++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> @@ -27,6 +27,8 @@ struct cpuidle_monitor *all_monitors[] = {
>   0
>   };
>   
> +int cpu_count;
> +
>   static struct cpuidle_monitor *monitors[MONITORS_MAX];
>   static unsigned int avail_monitors;
>   
> diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
> index 5b5eb1da0cce..c559d3115330 100644
> --- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
> +++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
> @@ -25,7 +25,7 @@
>   #endif
>   #define CSTATE_DESC_LEN 60
>   
> -int cpu_count;
> +extern int cpu_count;
>   
>   /* Hard to define the right names ...: */
>   enum power_range_e {
> 

thanks,
-- Shuah
Mike Gilbert Feb. 25, 2020, 5:48 p.m. UTC | #2
On Mon, Feb 24, 2020 at 5:26 PM shuah <shuah@kernel.org> wrote:
>
> Hi Mike,
>
> On 2/24/20 1:20 PM, Mike Gilbert wrote:
> > The -fno-common option will be enabled by default in GCC 10.
>
> Great. It landed in my inbox now.
> Please include the message you are seeing in the change log.

Sure, I will roll a v3 shortly.

> > diff --git a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
> > index 33dc34db4f3c..20f46348271b 100644
> > --- a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
> > +++ b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
> > @@ -82,7 +82,7 @@ static struct pci_access *pci_acc;
> >   static struct pci_dev *amd_fam14h_pci_dev;
> >   static int nbp1_entered;
> >
> > -struct timespec start_time;
> > +static struct timespec start_time;
> >   static unsigned long long timediff;
>
> Does it make sense to move start_time and timediff defines to
> cpupower-monitor.c and adding externs for them in
> cpupower-monitor.h?

I don't think that makes a lot of sense. start_time is really an
internal value and does not need to be shared between the two object
files.
Shuah Feb. 25, 2020, 9:51 p.m. UTC | #3
On 2/25/20 10:48 AM, Mike Gilbert wrote:
> On Mon, Feb 24, 2020 at 5:26 PM shuah <shuah@kernel.org> wrote:
>>
>> Hi Mike,
>>
>> On 2/24/20 1:20 PM, Mike Gilbert wrote:
>>> The -fno-common option will be enabled by default in GCC 10.
>>
>> Great. It landed in my inbox now.
>> Please include the message you are seeing in the change log.
> 
> Sure, I will roll a v3 shortly.

Great.

> 
>>> diff --git a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
>>> index 33dc34db4f3c..20f46348271b 100644
>>> --- a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
>>> +++ b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
>>> @@ -82,7 +82,7 @@ static struct pci_access *pci_acc;
>>>    static struct pci_dev *amd_fam14h_pci_dev;
>>>    static int nbp1_entered;
>>>
>>> -struct timespec start_time;
>>> +static struct timespec start_time;
>>>    static unsigned long long timediff;
>>
>> Does it make sense to move start_time and timediff defines to
>> cpupower-monitor.c and adding externs for them in
>> cpupower-monitor.h?
> 
> I don't think that makes a lot of sense. start_time is really an
> internal value and does not need to be shared between the two object
> files.
> 

Makes sense.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
index 33dc34db4f3c..20f46348271b 100644
--- a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
+++ b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
@@ -82,7 +82,7 @@  static struct pci_access *pci_acc;
 static struct pci_dev *amd_fam14h_pci_dev;
 static int nbp1_entered;
 
-struct timespec start_time;
+static struct timespec start_time;
 static unsigned long long timediff;
 
 #ifdef DEBUG
diff --git a/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c b/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
index 3c4cee160b0e..a65f7d011513 100644
--- a/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
+++ b/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
@@ -19,7 +19,7 @@  struct cpuidle_monitor cpuidle_sysfs_monitor;
 
 static unsigned long long **previous_count;
 static unsigned long long **current_count;
-struct timespec start_time;
+static struct timespec start_time;
 static unsigned long long timediff;
 
 static int cpuidle_get_count_percent(unsigned int id, double *percent,
diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
index 6d44fec55ad5..7c77045fef52 100644
--- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
+++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
@@ -27,6 +27,8 @@  struct cpuidle_monitor *all_monitors[] = {
 0
 };
 
+int cpu_count;
+
 static struct cpuidle_monitor *monitors[MONITORS_MAX];
 static unsigned int avail_monitors;
 
diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
index 5b5eb1da0cce..c559d3115330 100644
--- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
+++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
@@ -25,7 +25,7 @@ 
 #endif
 #define CSTATE_DESC_LEN 60
 
-int cpu_count;
+extern int cpu_count;
 
 /* Hard to define the right names ...: */
 enum power_range_e {