* [PATCH] powercap/intel_rapl: fix and tidy up error handling
@ 2016-11-23 19:01 Jacob Pan
2016-11-24 21:06 ` Rafael J. Wysocki
0 siblings, 1 reply; 5+ messages in thread
From: Jacob Pan @ 2016-11-23 19:01 UTC (permalink / raw)
To: LKML, Linux PM
Cc: Rafael Wysocki, Marcin Koss, Lukasz Odzioba, Thomas Gleixner,
Sebastian Andrzej Siewior, Jacob Pan
Commit e1399ba20eee ("powercap / RAPL: handle missing MSRs") added
contraint_to_pl() function to return index into an array. But it
can potentially return -EINVAL if powercap layer sends an out of
range constraint ID. This patch adds sanity check.
Unnecessary RAPL domain pointer check is removed since it must be
initialized before calling rapl_unit_xlate().
Reported-by: Odzioba, Lukasz <lukasz.odzioba@intel.com>
Reported-by: Koss, Marcin <marcin.koss@intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/powercap/intel_rapl.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index 65ed88a..f6937d0 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -429,6 +429,7 @@ static int contraint_to_pl(struct rapl_domain *rd, int cid)
return i;
}
}
+ pr_err("Cannot find matching power limit for constraint %d\n", cid);
return -EINVAL;
}
@@ -444,6 +445,10 @@ static int set_power_limit(struct powercap_zone *power_zone, int cid,
get_online_cpus();
rd = power_zone_to_rapl_domain(power_zone);
id = contraint_to_pl(rd, cid);
+ if (id < 0) {
+ ret = id;
+ goto set_exit;
+ }
rp = rd->rp;
@@ -483,6 +488,11 @@ static int get_current_power_limit(struct powercap_zone *power_zone, int cid,
get_online_cpus();
rd = power_zone_to_rapl_domain(power_zone);
id = contraint_to_pl(rd, cid);
+ if (id < 0) {
+ ret = id;
+ goto get_exit;
+ }
+
switch (rd->rpl[id].prim_id) {
case PL1_ENABLE:
prim = POWER_LIMIT1;
@@ -499,6 +509,7 @@ static int get_current_power_limit(struct powercap_zone *power_zone, int cid,
else
*data = val;
+get_exit:
put_online_cpus();
return ret;
@@ -514,6 +525,10 @@ static int set_time_window(struct powercap_zone *power_zone, int cid,
get_online_cpus();
rd = power_zone_to_rapl_domain(power_zone);
id = contraint_to_pl(rd, cid);
+ if (id < 0) {
+ ret = id;
+ goto set_time_exit;
+ }
switch (rd->rpl[id].prim_id) {
case PL1_ENABLE:
@@ -525,6 +540,8 @@ static int set_time_window(struct powercap_zone *power_zone, int cid,
default:
ret = -EINVAL;
}
+
+set_time_exit:
put_online_cpus();
return ret;
}
@@ -539,6 +556,10 @@ static int get_time_window(struct powercap_zone *power_zone, int cid, u64 *data)
get_online_cpus();
rd = power_zone_to_rapl_domain(power_zone);
id = contraint_to_pl(rd, cid);
+ if (id < 0) {
+ ret = id;
+ goto get_time_exit;
+ }
switch (rd->rpl[id].prim_id) {
case PL1_ENABLE:
@@ -553,6 +574,8 @@ static int get_time_window(struct powercap_zone *power_zone, int cid, u64 *data)
}
if (!ret)
*data = val;
+
+get_time_exit:
put_online_cpus();
return ret;
@@ -694,7 +717,7 @@ static u64 rapl_unit_xlate(struct rapl_domain *rd, enum unit_type type,
case ENERGY_UNIT:
scale = ENERGY_UNIT_SCALE;
/* per domain unit takes precedence */
- if (rd && rd->domain_energy_unit)
+ if (rd->domain_energy_unit)
units = rd->domain_energy_unit;
else
units = rp->energy_unit;
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] powercap/intel_rapl: fix and tidy up error handling
2016-11-23 19:01 [PATCH] powercap/intel_rapl: fix and tidy up error handling Jacob Pan
@ 2016-11-24 21:06 ` Rafael J. Wysocki
2016-11-28 17:31 ` Jacob Pan
0 siblings, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki @ 2016-11-24 21:06 UTC (permalink / raw)
To: Jacob Pan
Cc: LKML, Linux PM, Rafael Wysocki, Marcin Koss, Lukasz Odzioba,
Thomas Gleixner, Sebastian Andrzej Siewior
On Wed, Nov 23, 2016 at 8:01 PM, Jacob Pan
<jacob.jun.pan@linux.intel.com> wrote:
> Commit e1399ba20eee ("powercap / RAPL: handle missing MSRs") added
> contraint_to_pl() function to return index into an array. But it
> can potentially return -EINVAL if powercap layer sends an out of
> range constraint ID. This patch adds sanity check.
>
> Unnecessary RAPL domain pointer check is removed since it must be
> initialized before calling rapl_unit_xlate().
>
> Reported-by: Odzioba, Lukasz <lukasz.odzioba@intel.com>
> Reported-by: Koss, Marcin <marcin.koss@intel.com>
Does this depend on the Thomas' series only?
Also, it looks like a Fixes: tag should be present here.
>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Thanks,
Rafael
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powercap/intel_rapl: fix and tidy up error handling
2016-11-24 21:06 ` Rafael J. Wysocki
@ 2016-11-28 17:31 ` Jacob Pan
2016-11-28 21:54 ` Jacob Pan
0 siblings, 1 reply; 5+ messages in thread
From: Jacob Pan @ 2016-11-28 17:31 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: LKML, Linux PM, Rafael Wysocki, Marcin Koss, Lukasz Odzioba,
Thomas Gleixner, Sebastian Andrzej Siewior, jacob.jun.pan
On Thu, 24 Nov 2016 22:06:59 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:
> On Wed, Nov 23, 2016 at 8:01 PM, Jacob Pan
> <jacob.jun.pan@linux.intel.com> wrote:
> > Commit e1399ba20eee ("powercap / RAPL: handle missing MSRs") added
> > contraint_to_pl() function to return index into an array. But it
> > can potentially return -EINVAL if powercap layer sends an out of
> > range constraint ID. This patch adds sanity check.
> >
> > Unnecessary RAPL domain pointer check is removed since it must be
> > initialized before calling rapl_unit_xlate().
> >
> > Reported-by: Odzioba, Lukasz <lukasz.odzioba@intel.com>
> > Reported-by: Koss, Marcin <marcin.koss@intel.com>
>
> Does this depend on the Thomas' series only?
>
yes, it is on top of Thomas' series.
> Also, it looks like a Fixes: tag should be present here.
>
OK. will add that.
> >
> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>
> Thanks,
> Rafael
[Jacob Pan]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powercap/intel_rapl: fix and tidy up error handling
2016-11-28 17:31 ` Jacob Pan
@ 2016-11-28 21:54 ` Jacob Pan
0 siblings, 0 replies; 5+ messages in thread
From: Jacob Pan @ 2016-11-28 21:54 UTC (permalink / raw)
To: Jacob Pan
Cc: Rafael J. Wysocki, LKML, Linux PM, Rafael Wysocki, Marcin Koss,
Lukasz Odzioba, Thomas Gleixner, Sebastian Andrzej Siewior
On Mon, 28 Nov 2016 09:31:03 -0800
Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:
> > Also, it looks like a Fixes: tag should be present here.
> >
I just resent the patch with Fixes: tag.
Thanks,
Jacob
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] powercap/intel_rapl: fix and tidy up error handling
@ 2016-11-28 21:53 Jacob Pan
0 siblings, 0 replies; 5+ messages in thread
From: Jacob Pan @ 2016-11-28 21:53 UTC (permalink / raw)
To: LKML, Linux PM, Rafael Wysocki
Cc: Srinivas Pandruvada, Lukasz Odzioba, Marcin Koss,
Thomas Gleixner, Jacob Pan
Commit e1399ba20eee ("powercap / RAPL: handle missing MSRs") added
contraint_to_pl() function to return index into an array. But it
can potentially return -EINVAL if powercap layer sends an out of
range constraint ID. This patch adds sanity check.
Unnecessary RAPL domain pointer check is removed since it must be
initialized before calling rapl_unit_xlate().
Fixes: e1399ba20eee ("powercap / RAPL: handle missing MSRs")
Reported-by: Odzioba, Lukasz <lukasz.odzioba@intel.com>
Reported-by: Koss, Marcin <marcin.koss@intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/powercap/intel_rapl.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index 65ed88a..f6937d0 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -429,6 +429,7 @@ static int contraint_to_pl(struct rapl_domain *rd, int cid)
return i;
}
}
+ pr_err("Cannot find matching power limit for constraint %d\n", cid);
return -EINVAL;
}
@@ -444,6 +445,10 @@ static int set_power_limit(struct powercap_zone *power_zone, int cid,
get_online_cpus();
rd = power_zone_to_rapl_domain(power_zone);
id = contraint_to_pl(rd, cid);
+ if (id < 0) {
+ ret = id;
+ goto set_exit;
+ }
rp = rd->rp;
@@ -483,6 +488,11 @@ static int get_current_power_limit(struct powercap_zone *power_zone, int cid,
get_online_cpus();
rd = power_zone_to_rapl_domain(power_zone);
id = contraint_to_pl(rd, cid);
+ if (id < 0) {
+ ret = id;
+ goto get_exit;
+ }
+
switch (rd->rpl[id].prim_id) {
case PL1_ENABLE:
prim = POWER_LIMIT1;
@@ -499,6 +509,7 @@ static int get_current_power_limit(struct powercap_zone *power_zone, int cid,
else
*data = val;
+get_exit:
put_online_cpus();
return ret;
@@ -514,6 +525,10 @@ static int set_time_window(struct powercap_zone *power_zone, int cid,
get_online_cpus();
rd = power_zone_to_rapl_domain(power_zone);
id = contraint_to_pl(rd, cid);
+ if (id < 0) {
+ ret = id;
+ goto set_time_exit;
+ }
switch (rd->rpl[id].prim_id) {
case PL1_ENABLE:
@@ -525,6 +540,8 @@ static int set_time_window(struct powercap_zone *power_zone, int cid,
default:
ret = -EINVAL;
}
+
+set_time_exit:
put_online_cpus();
return ret;
}
@@ -539,6 +556,10 @@ static int get_time_window(struct powercap_zone *power_zone, int cid, u64 *data)
get_online_cpus();
rd = power_zone_to_rapl_domain(power_zone);
id = contraint_to_pl(rd, cid);
+ if (id < 0) {
+ ret = id;
+ goto get_time_exit;
+ }
switch (rd->rpl[id].prim_id) {
case PL1_ENABLE:
@@ -553,6 +574,8 @@ static int get_time_window(struct powercap_zone *power_zone, int cid, u64 *data)
}
if (!ret)
*data = val;
+
+get_time_exit:
put_online_cpus();
return ret;
@@ -694,7 +717,7 @@ static u64 rapl_unit_xlate(struct rapl_domain *rd, enum unit_type type,
case ENERGY_UNIT:
scale = ENERGY_UNIT_SCALE;
/* per domain unit takes precedence */
- if (rd && rd->domain_energy_unit)
+ if (rd->domain_energy_unit)
units = rd->domain_energy_unit;
else
units = rp->energy_unit;
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-28 21:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-23 19:01 [PATCH] powercap/intel_rapl: fix and tidy up error handling Jacob Pan
2016-11-24 21:06 ` Rafael J. Wysocki
2016-11-28 17:31 ` Jacob Pan
2016-11-28 21:54 ` Jacob Pan
2016-11-28 21:53 Jacob Pan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).