[PATCH -next v2] power/qos: annotate data races in pm_qos_*_value

[PATCH -next v2] power/qos: annotate data races in pm_qos_*_value

[Qian Cai]

The target_value field in struct pm_qos_constraints is used for lockless
access to the effective constraint value of a given QoS list, so the
readers of it cannot expect it to always reflect the most recent
effective constraint value.  However, they can and do expect it to be
equal to a valid effective constraint value computed at a certain time
in the past (event though it may not be the most recent one), so add
READ|WRITE_ONCE() annotations around the target_value accesses to
prevent the compiler from possibly causing that expectation to be unmet
by generating code in an exceptionally convoluted way.

Signed-off-by: Qian Cai <cai@lca.pw>
---

v2: borrow the commit log from Rafael.

 kernel/power/qos.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 32927682bcc4..db0bed2cae26 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -52,7 +52,7 @@ static DEFINE_SPINLOCK(pm_qos_lock);
  */
 s32 pm_qos_read_value(struct pm_qos_constraints *c)
 {
-	return c->target_value;
+	return READ_ONCE(c->target_value);
 }
 
 static int pm_qos_get_value(struct pm_qos_constraints *c)
@@ -75,7 +75,7 @@ static int pm_qos_get_value(struct pm_qos_constraints *c)
 
 static void pm_qos_set_value(struct pm_qos_constraints *c, s32 value)
 {
-	c->target_value = value;
+	WRITE_ONCE(c->target_value, value);
 }
 
 /**
-- 
2.21.0 (Apple Git-122.2)

Re: [PATCH -next v2] power/qos: annotate data races in pm_qos_*_value

[Rafael J. Wysocki]

On Wed, Feb 26, 2020 at 2:58 AM Qian Cai <cai@lca.pw> wrote:
>
> The target_value field in struct pm_qos_constraints is used for lockless
> access to the effective constraint value of a given QoS list, so the
> readers of it cannot expect it to always reflect the most recent
> effective constraint value.  However, they can and do expect it to be
> equal to a valid effective constraint value computed at a certain time
> in the past (event though it may not be the most recent one), so add
> READ|WRITE_ONCE() annotations around the target_value accesses to
> prevent the compiler from possibly causing that expectation to be unmet
> by generating code in an exceptionally convoluted way.
>
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
>
> v2: borrow the commit log from Rafael.
>
>  kernel/power/qos.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 32927682bcc4..db0bed2cae26 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -52,7 +52,7 @@ static DEFINE_SPINLOCK(pm_qos_lock);
>   */
>  s32 pm_qos_read_value(struct pm_qos_constraints *c)
>  {
> -       return c->target_value;
> +       return READ_ONCE(c->target_value);
>  }
>
>  static int pm_qos_get_value(struct pm_qos_constraints *c)
> @@ -75,7 +75,7 @@ static int pm_qos_get_value(struct pm_qos_constraints *c)
>
>  static void pm_qos_set_value(struct pm_qos_constraints *c, s32 value)
>  {
> -       c->target_value = value;
> +       WRITE_ONCE(c->target_value, value);
>  }
>
>  /**
> --

Applied as 5.7 material, thanks!