On Fri, 22 Jan 2016 12:49:05 +0530
Shilpasri G Bhat <shilpa.b...@linux.vnet.ibm.com> wrote:

> Create sysfs attributes to export throttle information in
> /sys/devices/system/cpu/cpufreq/chipN. The newly added sysfs files are as
> follows:
> 
> 1)/sys/devices/system/cpu/cpufreq/chip0/throttle_frequencies
>   This gives the throttle stats for each of the available frequencies.
>   The throttle stat of a frequency is the total number of times the max
>   frequency is reduced to that frequency.
>   # cat /sys/devices/system/cpu/cpufreq/chip0/throttle_frequencies
>   4023000 0
>   3990000 0
>   3956000 1
>   3923000 0
>   3890000 0
>   3857000 2
>   3823000 0
>   3790000 0
>   3757000 2
>   3724000 1
>   3690000 1
>   ...
> 
> 2)/sys/devices/system/cpu/cpufreq/chip0/throttle_reasons
>   This directory contains throttle reason files. Each file gives the
>   total number of times the max frequency is throttled, except for
>   'throttle_reset', which gives the total number of times the max

Is reset a good name? Ideally a reset, reset's stats.

>   frequency is unthrottled after being throttled.
>   # cd /sys/devices/system/cpu/cpufreq/chip0/throttle_reasons
>   # cat cpu_over_temperature
>   7
>   # cat occ_reset
>   0
>   # cat over_current
>   0
>   # cat power_cap
>   0
>   # cat power_supply_failure
>   0
>   # cat throttle_reset
>   7
> 
> 3)/sys/devices/system/cpu/cpufreq/chip0/throttle_stat
>   This gives the total number of events of max frequency throttling to
>   lower frequencies in the turbo range of frequencies and the sub-turbo(at
>   and below nominal) range of frequencies.
>   # cat /sys/devices/system/cpu/cpufreq/chip0/throttle_stat
>   turbo 7
>   sub-turbo 0
> 
> Signed-off-by: Shilpasri G Bhat <shilpa.b...@linux.vnet.ibm.com>
> Reviewed-by: Gautham R. Shenoy <e...@linux.vnet.ibm.com>
snip

>  
> +enum throt_reason_type {
> +     NO_THROTTLE = 0,
NO is not throttled or number_throttle?

> +     POWERCAP,
> +     CPU_OVERTEMP,
> +     POWER_SUPPLY_FAILURE,
> +     OVERCURRENT,
> +     OCC_RESET_THROTTLE,
> +     OCC_MAX_REASON
> +};
> +
>  static struct chip {
>       unsigned int id;
>       bool throttled;
> @@ -62,6 +72,11 @@ static struct chip {
>       u8 throt_reason;
>       cpumask_t mask;
>       struct work_struct throttle;
> +     int throt_turbo;

The enum uses _THROTTLE so why can't the struct member
be throttle_nominal? throttle_turbo?

> +     int throt_nominal;
> +     int reason[OCC_MAX_REASON];
> +     int *pstate_stat;
> +     struct kobject *kobj;
>  } *chips;
>  
>  static int nr_chips;
> @@ -196,6 +211,128 @@ static struct freq_attr *powernv_cpu_freq_attr[] = {
>       NULL,
>  };
>  
> +static inline int get_chip_index(unsigned int id)
> +{
> +     int i;
> +
> +     for (i = 0; i < nr_chips; i++)
> +             if (chips[i].id == id)
> +                     return i;
> +
> +     return -EINVAL;
> +}
> +
> +static ssize_t throttle_freq_show(struct kobject *kobj,
> +                               struct kobj_attribute *attr, char *buf)
> +{
> +     int i, count = 0, id;
> +
> +     i = kstrtoint(kobj->name + 4, 0, &id);

Why the +4 magic, make it more readable?

> +     if (i)
> +             return i;
> +
> +     id = get_chip_index(id);
> +     if (id < 0) {
> +             pr_warn_once("%s Matching chip-id not found\n", __func__);

The pr_warn_once should also print which chip-id was not found, please
add that to the print

> +             return id;
> +     }
> +
> +     for (i = 0; i < powernv_pstate_info.nr_pstates; i++)
> +             count += sprintf(&buf[count], "%d %d\n",
> +                            powernv_freqs[i].frequency,
> +                            chips[id].pstate_stat[i]);
> +
> +     return count;
> +}
> +
> +static struct kobj_attribute attr_throttle_frequencies =
> +__ATTR(throttle_frequencies, 0444, throttle_freq_show, NULL);
> +
> +static ssize_t throttle_stat_show(struct kobject *kobj,
> +                               struct kobj_attribute *attr, char *buf)
> +{
> +     int ret, id, count = 0;
> +
> +     ret = kstrtoint(kobj->name + 4, 0, &id);
> +     if (ret)
> +             return ret;
> +
> +     id = get_chip_index(id);
> +     if (id < 0) {
> +             pr_warn_once("%s Matching chip-id not found\n", __func__);
> +             return id;
> +     }
> +

The above 9 lines look like common code, you can easily collapse it
instead of repeating it

> +     count += sprintf(&buf[count], "turbo %d\n", chips[id].throt_turbo);
> +     count += sprintf(&buf[count], "sub-turbo %d\n",
> +                                     chips[id].throt_nominal);
> +
> +     return count;
> +}
> +
> +static struct kobj_attribute attr_throttle_stat =
> +__ATTR(throttle_stat, 0444, throttle_stat_show, NULL);
> +
> +#define define_throttle_reason_attr(attr_name, val)                    \
> +static ssize_t attr_name##_show(struct kobject *kobj,                        
>   \
> +                               struct kobj_attribute *attr, char *buf) \
> +{                                                                      \
> +     int ret, id;                                                      \
> +                                                                       \
> +     ret = kstrtoint(kobj->name + 4, 0, &id);                          \
> +     if (ret)                                                          \
> +             return ret;                                               \
> +                                                                       \
> +     id = get_chip_index(id);                                          \
> +     if (id < 0) {                                                     \
> +             pr_warn_once("%s Matching chip-id not found\n", __func__);\
> +             return id;                                                \
> +     }                                                                 \
> +                                                                       \
> +     return sprintf(buf, "%d\n", chips[id].reason[val]);               \
> +}                                                                      \
> +                                                                       \
> +static struct kobj_attribute attr_##attr_name =                              
>   \
> +__ATTR(attr_name, 0444, attr_name##_show, NULL)
> +
> +define_throttle_reason_attr(throttle_reset, NO_THROTTLE);
> +define_throttle_reason_attr(power_cap, POWERCAP);
> +define_throttle_reason_attr(cpu_over_temperature, CPU_OVERTEMP);
> +define_throttle_reason_attr(power_supply_failure, POWER_SUPPLY_FAILURE);
> +define_throttle_reason_attr(over_current, OVERCURRENT);
> +define_throttle_reason_attr(occ_reset, OCC_RESET_THROTTLE);
> +
> +static struct attribute *throttle_reason_attrs[] = {
> +     &attr_throttle_reset.attr,
> +     &attr_power_cap.attr,
> +     &attr_cpu_over_temperature.attr,
> +     &attr_power_supply_failure.attr,
> +     &attr_over_current.attr,
> +     &attr_occ_reset.attr,
> +     NULL
> +};
> +
> +static struct attribute *throttle_stat_attrs[] = {
> +     &attr_throttle_frequencies.attr,
> +     &attr_throttle_stat.attr,
> +     NULL
> +};
> +
> +static const struct attribute_group throttle_reason_group = {
> +     .name   = "throttle_reasons",
> +     .attrs  = throttle_reason_attrs,
> +};
> +
> +static const struct attribute_group throttle_stat_group = {
> +     .attrs = throttle_stat_attrs,
> +};
> +
> +static const struct attribute_group *throttle_attr_groups[] = {
> +     &throttle_stat_group,
> +     &throttle_reason_group,
> +     NULL
> +};
> +
>  /* Helper routines */
>  
>  /* Access helpers to power mgt SPR */
> @@ -327,13 +464,15 @@ static void powernv_cpufreq_throttle_check(void *data)
>       unsigned int cpu = smp_processor_id();
>       unsigned int chip_id = core_to_chip_map[cpu_core_index_of_thread(cpu)];
>       unsigned long pmsr;
> -     int pmsr_pmax, i;
> +     int pmsr_pmax, i, index;
>  
>       pmsr = get_pmspr(SPRN_PMSR);
>  
> -     for (i = 0; i < nr_chips; i++)
> -             if (chips[i].id == chip_id)
> -                     break;
> +     i = get_chip_index(chip_id);
> +     if (unlikely(i < 0)) {
> +             pr_warn_once("%s Matching chip-id not found\n", __func__);
> +             return;
> +     }
>  
>       /* Check for Pmax Capping */
>       pmsr_pmax = (s8)PMSR_MAX(pmsr);
> @@ -341,10 +480,19 @@ static void powernv_cpufreq_throttle_check(void *data)
>               if (chips[i].throttled)
>                       goto next;
>               chips[i].throttled = true;
> -             if (pmsr_pmax < powernv_pstate_info.nominal)
> +             if (pmsr_pmax < powernv_pstate_info.nominal) {
>                       pr_warn_once("CPU %d on Chip %u has Pmax reduced below 
> nominal frequency (%d < %d)\n",
>                                    cpu, chips[i].id, pmsr_pmax,
>                                    powernv_pstate_info.nominal);
> +                     chips[i].throt_nominal++;
> +             } else {
> +                     chips[i].throt_turbo++;
> +             }
> +
> +             index  = powernv_pstate_info.max - pmsr_pmax;
> +             if (index >= 0 && index < powernv_pstate_info.nr_pstates)
> +                     chips[i].pstate_stat[index]++;
> +
>               trace_powernv_throttle(chips[i].id,
>                                      throttle_reason[chips[i].throt_reason],
>                                      pmsr_pmax);
> @@ -512,13 +660,18 @@ static int powernv_cpufreq_occ_msg(struct 
> notifier_block *nb,
>                       return 0;
>               }
>  
> -             for (i = 0; i < nr_chips; i++)
> -                     if (chips[i].id == omsg.chip)
> -                             break;
> +             i = get_chip_index(omsg.chip);
> +             if (i < 0) {
> +                     pr_warn_once("%s Matching chip-id not found\n",
> +                                  __func__);
> +                     return i;
> +             }
>  
>               if (omsg.throttle_status >= 0 &&
> -                 omsg.throttle_status <= OCC_MAX_THROTTLE_STATUS)
> +                 omsg.throttle_status <= OCC_MAX_THROTTLE_STATUS) {
>                       chips[i].throt_reason = omsg.throttle_status;
> +                     chips[i].reason[omsg.throttle_status]++;
> +             }
>  
>               if (!omsg.throttle_status)
>                       chips[i].restore = true;
> @@ -583,12 +736,38 @@ static int init_chip_info(void)
>               goto free_chip_map;
>  
>       for (i = 0; i < nr_chips; i++) {
> +             char name[10];
> +
>               chips[i].id = chip[i];
>               cpumask_copy(&chips[i].mask, cpumask_of_node(chip[i]));
>               INIT_WORK(&chips[i].throttle, powernv_cpufreq_work_fn);
> +             chips[i].pstate_stat = kcalloc(powernv_pstate_info.nr_pstates,
> +                                             sizeof(int), GFP_KERNEL);
> +             if (!chips[i].pstate_stat)
> +                     goto free;
> +
> +             sprintf(name, "chip%d", chips[i].id);
> +             chips[i].kobj = kobject_create_and_add(name,
> +                                                    cpufreq_global_kobject);
> +             if (!chips[i].kobj)
> +                     goto free;
> +
> +             ret = sysfs_create_groups(chips[i].kobj, throttle_attr_groups);
> +             if (ret) {
> +                     pr_info("Chip %d failed to create throttle sysfs 
> group\n",
> +                             chips[i].id);
> +                     goto free;
> +             }
>       }
>  
>       return 0;
> +free:
> +     nr_chips = i;
> +     for (i = 0; i <= nr_chips; i++) {
> +             kobject_put(chips[i].kobj);
> +             kfree(chips[i].pstate_stat);
> +     }
> +     kfree(chips);
>  free_chip_map:
>       kfree(core_to_chip_map);
>  out:
> @@ -623,9 +802,17 @@ module_init(powernv_cpufreq_init);
>  
>  static void __exit powernv_cpufreq_exit(void)
>  {
> +     int i;
> +
>       unregister_reboot_notifier(&powernv_cpufreq_reboot_nb);
>       opal_message_notifier_unregister(OPAL_MSG_OCC,
>                                        &powernv_cpufreq_opal_nb);
> +
> +     for (i = 0; i < nr_chips; i++) {
> +             kobject_put(chips[i].kobj);
> +             kfree(chips[i].pstate_stat);
> +     }
> +
>       kfree(chips);
>       kfree(core_to_chip_map);
>       cpufreq_unregister_driver(&powernv_cpufreq_driver);

Reply via email to