When the current uncore frequency can't be read, don't create attribute
"current_freq_khz" as any read will fail later. Some user space
applications like turbostat fail to continue with the failure. So, check
error during attribute creation.
Fixes: 414eef27283a ("platform/x86/intel/uncore-freq: Display uncore current frequency")
Signed-off-by: Srinivas Pandruvada <[email protected]>
---
Added fixes tag which added current_freq_khz. But after this
tag there is reorg of code, so need manual backport for some versions.
I will separately submit to stable trees after merge.
.../x86/intel/uncore-frequency/uncore-frequency-common.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
index 1152deaa0078..33ab207493e3 100644
--- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
+++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
@@ -176,7 +176,7 @@ show_uncore_data(initial_max_freq_khz);
static int create_attr_group(struct uncore_data *data, char *name)
{
- int ret, index = 0;
+ int ret, freq, index = 0;
init_attribute_rw(max_freq_khz);
init_attribute_rw(min_freq_khz);
@@ -197,7 +197,11 @@ static int create_attr_group(struct uncore_data *data, char *name)
data->uncore_attrs[index++] = &data->min_freq_khz_dev_attr.attr;
data->uncore_attrs[index++] = &data->initial_min_freq_khz_dev_attr.attr;
data->uncore_attrs[index++] = &data->initial_max_freq_khz_dev_attr.attr;
- data->uncore_attrs[index++] = &data->current_freq_khz_dev_attr.attr;
+
+ ret = uncore_read_freq(data, &freq);
+ if (!ret)
+ data->uncore_attrs[index++] = &data->current_freq_khz_dev_attr.attr;
+
data->uncore_attrs[index] = NULL;
data->uncore_attr_group.name = name;
--
2.41.0
On Thu, Oct 05, 2023 at 09:03:20AM +0200, Hans de Goede wrote:
> On 10/4/23 20:19, Srinivas Pandruvada wrote:
…
> Can you for future updated patches please use the ususal [PATCH v2],
> [PATCH v3], etc. prefix ?
Side note: the `git format-patch ...` has -vX option, where X is a version.
--
With Best Regards,
Andy Shevchenko
Hi Hans,
On Thu, 2023-10-05 at 09:03 +0200, Hans de Goede wrote:
> Hi Srinivas,
>
> On 10/4/23 20:19, Srinivas Pandruvada wrote:
> > When the current uncore frequency can't be read, don't create
> > attribute
> > "current_freq_khz" as any read will fail later. Some user space
> > applications like turbostat fail to continue with the failure. So,
> > check
> > error during attribute creation.
> >
> > Fixes: 414eef27283a ("platform/x86/intel/uncore-freq: Display
> > uncore current frequency")
> > Signed-off-by: Srinivas Pandruvada
> > <[email protected]>
> > ---
> > Added fixes tag which added current_freq_khz. But after this
> > tag there is reorg of code, so need manual backport for some
> > versions.
> > I will separately submit to stable trees after merge.
>
> Can you for future updated patches please use the ususal [PATCH v2],
> [PATCH v3], etc. prefix ?
>
Sorry about this. I usually do with code change. But will keep in mind.
Thanks,
Srinivas
> Also please document the changes per version after the cutline, e.g.
> :
>
> ```
> ---
> Changes in v2:
> - Added fixes tag which added current_freq_khz. Note after this
> tag there is reorg of code, so need manual backport for some
> versions.
> I will separately submit to stable trees after merge.
> ```
>
> Regards,
>
> Hans
>
>
>
>
> >
> > .../x86/intel/uncore-frequency/uncore-frequency-common.c | 8
> > ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-
> > frequency-common.c b/drivers/platform/x86/intel/uncore-
> > frequency/uncore-frequency-common.c
> > index 1152deaa0078..33ab207493e3 100644
> > --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
> > common.c
> > +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
> > common.c
> > @@ -176,7 +176,7 @@ show_uncore_data(initial_max_freq_khz);
> >
> > static int create_attr_group(struct uncore_data *data, char *name)
> > {
> > - int ret, index = 0;
> > + int ret, freq, index = 0;
> >
> > init_attribute_rw(max_freq_khz);
> > init_attribute_rw(min_freq_khz);
> > @@ -197,7 +197,11 @@ static int create_attr_group(struct
> > uncore_data *data, char *name)
> > data->uncore_attrs[index++] = &data-
> > >min_freq_khz_dev_attr.attr;
> > data->uncore_attrs[index++] = &data-
> > >initial_min_freq_khz_dev_attr.attr;
> > data->uncore_attrs[index++] = &data-
> > >initial_max_freq_khz_dev_attr.attr;
> > - data->uncore_attrs[index++] = &data-
> > >current_freq_khz_dev_attr.attr;
> > +
> > + ret = uncore_read_freq(data, &freq);
> > + if (!ret)
> > + data->uncore_attrs[index++] = &data-
> > >current_freq_khz_dev_attr.attr;
> > +
> > data->uncore_attrs[index] = NULL;
> >
> > data->uncore_attr_group.name = name;
>
Hi Srinivas,
On 10/4/23 20:19, Srinivas Pandruvada wrote:
> When the current uncore frequency can't be read, don't create attribute
> "current_freq_khz" as any read will fail later. Some user space
> applications like turbostat fail to continue with the failure. So, check
> error during attribute creation.
>
> Fixes: 414eef27283a ("platform/x86/intel/uncore-freq: Display uncore current frequency")
> Signed-off-by: Srinivas Pandruvada <[email protected]>
> ---
> Added fixes tag which added current_freq_khz. But after this
> tag there is reorg of code, so need manual backport for some versions.
> I will separately submit to stable trees after merge.
Can you for future updated patches please use the ususal [PATCH v2],
[PATCH v3], etc. prefix ?
Also please document the changes per version after the cutline, e.g. :
```
---
Changes in v2:
- Added fixes tag which added current_freq_khz. Note after this
tag there is reorg of code, so need manual backport for some versions.
I will separately submit to stable trees after merge.
```
Regards,
Hans
>
> .../x86/intel/uncore-frequency/uncore-frequency-common.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> index 1152deaa0078..33ab207493e3 100644
> --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> @@ -176,7 +176,7 @@ show_uncore_data(initial_max_freq_khz);
>
> static int create_attr_group(struct uncore_data *data, char *name)
> {
> - int ret, index = 0;
> + int ret, freq, index = 0;
>
> init_attribute_rw(max_freq_khz);
> init_attribute_rw(min_freq_khz);
> @@ -197,7 +197,11 @@ static int create_attr_group(struct uncore_data *data, char *name)
> data->uncore_attrs[index++] = &data->min_freq_khz_dev_attr.attr;
> data->uncore_attrs[index++] = &data->initial_min_freq_khz_dev_attr.attr;
> data->uncore_attrs[index++] = &data->initial_max_freq_khz_dev_attr.attr;
> - data->uncore_attrs[index++] = &data->current_freq_khz_dev_attr.attr;
> +
> + ret = uncore_read_freq(data, &freq);
> + if (!ret)
> + data->uncore_attrs[index++] = &data->current_freq_khz_dev_attr.attr;
> +
> data->uncore_attrs[index] = NULL;
>
> data->uncore_attr_group.name = name;
On Thu, 5 Oct 2023, srinivas pandruvada wrote:
> On Thu, 2023-10-05 at 09:03 +0200, Hans de Goede wrote:
> > Hi Srinivas,
> >
> > On 10/4/23 20:19, Srinivas Pandruvada wrote:
> > > When the current uncore frequency can't be read, don't create
> > > attribute
> > > "current_freq_khz" as any read will fail later. Some user space
> > > applications like turbostat fail to continue with the failure. So,
> > > check
> > > error during attribute creation.
> > >
> > > Fixes: 414eef27283a ("platform/x86/intel/uncore-freq: Display
> > > uncore current frequency")
> > > Signed-off-by: Srinivas Pandruvada
> > > <[email protected]>
> > > ---
> > > Added fixes tag which added current_freq_khz. But after this
> > > tag there is reorg of code, so need manual backport for some
> > > versions.
> > > I will separately submit to stable trees after merge.
> >
> > Can you for future updated patches please use the ususal [PATCH v2],
> > [PATCH v3], etc. prefix ?
> >
> Sorry about this. I usually do with code change. But will keep in mind.
Reviewed-by: Ilpo Järvinen <[email protected]>
--
i.
Hi,
On 10/4/23 20:19, Srinivas Pandruvada wrote:
> When the current uncore frequency can't be read, don't create attribute
> "current_freq_khz" as any read will fail later. Some user space
> applications like turbostat fail to continue with the failure. So, check
> error during attribute creation.
>
> Fixes: 414eef27283a ("platform/x86/intel/uncore-freq: Display uncore current frequency")
> Signed-off-by: Srinivas Pandruvada <[email protected]>
> Reviewed-by: Ilpo Järvinen <[email protected]>
> ---
> Added fixes tag which added current_freq_khz. But after this
> tag there is reorg of code, so need manual backport for some versions.
> I will separately submit to stable trees after merge.
Thank you for your patch/series, I've applied this patch
(series) to the pdx86 fixes branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes
Note it will show up in the pdx86 fixes branch once I've pushed
my local branch there, which might take a while.
I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.
Regards,
Hans
>
> .../x86/intel/uncore-frequency/uncore-frequency-common.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> index 1152deaa0078..33ab207493e3 100644
> --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> @@ -176,7 +176,7 @@ show_uncore_data(initial_max_freq_khz);
>
> static int create_attr_group(struct uncore_data *data, char *name)
> {
> - int ret, index = 0;
> + int ret, freq, index = 0;
>
> init_attribute_rw(max_freq_khz);
> init_attribute_rw(min_freq_khz);
> @@ -197,7 +197,11 @@ static int create_attr_group(struct uncore_data *data, char *name)
> data->uncore_attrs[index++] = &data->min_freq_khz_dev_attr.attr;
> data->uncore_attrs[index++] = &data->initial_min_freq_khz_dev_attr.attr;
> data->uncore_attrs[index++] = &data->initial_max_freq_khz_dev_attr.attr;
> - data->uncore_attrs[index++] = &data->current_freq_khz_dev_attr.attr;
> +
> + ret = uncore_read_freq(data, &freq);
> + if (!ret)
> + data->uncore_attrs[index++] = &data->current_freq_khz_dev_attr.attr;
> +
> data->uncore_attrs[index] = NULL;
>
> data->uncore_attr_group.name = name;