linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] thermal: netlink: Improve the initcall ordering
@ 2020-07-17 16:42 Daniel Lezcano
  2020-07-17 16:42 ` [PATCH 2/2] thermal: core: Move initialization after core initcall Daniel Lezcano
  2020-07-20  5:43 ` [PATCH 1/2] thermal: netlink: Improve the initcall ordering Amit Kucheria
  0 siblings, 2 replies; 4+ messages in thread
From: Daniel Lezcano @ 2020-07-17 16:42 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: linux-kernel, linux-pm, m.szyprowski, amit.kucheria

The initcalls like to play joke. In our case, the thermal-netlink
initcall is called after the thermal-core initcall but this one sends
a notification before the former is initialzed. No issue was spotted,
but it could lead to a memory corruption, so instead of relying on the
core_initcall for the thermal-netlink, let's initialize directly from
the thermal-core init routine, so we have full control of the init
ordering.

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/thermal_core.c    | 4 ++++
 drivers/thermal/thermal_netlink.c | 3 +--
 drivers/thermal/thermal_netlink.h | 6 ++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 25ef29123f72..c2e7d7aaa354 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1581,6 +1581,10 @@ static int __init thermal_init(void)
 {
 	int result;
 
+	result = thermal_netlink_init();
+	if (result)
+		goto error;
+
 	mutex_init(&poweroff_lock);
 	result = thermal_register_governors();
 	if (result)
diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
index dd0a3b889674..42eace7401da 100644
--- a/drivers/thermal/thermal_netlink.c
+++ b/drivers/thermal/thermal_netlink.c
@@ -641,8 +641,7 @@ static struct genl_family thermal_gnl_family __ro_after_init = {
 	.n_mcgrps	= ARRAY_SIZE(thermal_genl_mcgrps),
 };
 
-static int __init thermal_netlink_init(void)
+int __init thermal_netlink_init(void)
 {
 	return genl_register_family(&thermal_gnl_family);
 }
-core_initcall(thermal_netlink_init);
diff --git a/drivers/thermal/thermal_netlink.h b/drivers/thermal/thermal_netlink.h
index 0ec28d105da5..828d1dddfa98 100644
--- a/drivers/thermal/thermal_netlink.h
+++ b/drivers/thermal/thermal_netlink.h
@@ -6,6 +6,7 @@
 
 /* Netlink notification function */
 #ifdef CONFIG_THERMAL_NETLINK
+int __init thermal_netlink_init(void);
 int thermal_notify_tz_create(int tz_id, const char *name);
 int thermal_notify_tz_delete(int tz_id);
 int thermal_notify_tz_enable(int tz_id);
@@ -23,6 +24,11 @@ int thermal_notify_cdev_delete(int cdev_id);
 int thermal_notify_tz_gov_change(int tz_id, const char *name);
 int thermal_genl_sampling_temp(int id, int temp);
 #else
+static inline int thermal_netlink_init(void)
+{
+	return 0;
+}
+
 static inline int thermal_notify_tz_create(int tz_id, const char *name)
 {
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH 2/2] thermal: core: Move initialization after core initcall
  2020-07-17 16:42 [PATCH 1/2] thermal: netlink: Improve the initcall ordering Daniel Lezcano
@ 2020-07-17 16:42 ` Daniel Lezcano
  2020-07-20  5:39   ` Amit Kucheria
  2020-07-20  5:43 ` [PATCH 1/2] thermal: netlink: Improve the initcall ordering Amit Kucheria
  1 sibling, 1 reply; 4+ messages in thread
From: Daniel Lezcano @ 2020-07-17 16:42 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: linux-kernel, linux-pm, m.szyprowski, amit.kucheria

The generic netlink is initialized at subsys_initcall, so far after
the thermal init routine and the thermal generic netlink family
initialization.

On ŝome platforms, that leads to a memory corruption.

The fix was sent to netdev@ to move the genetlink framework
initialization at core_initcall.

Move the thermal core initialization to postcore level which is very
close to core level.

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/thermal_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index c2e7d7aaa354..79551bb6cd4c 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1617,4 +1617,4 @@ static int __init thermal_init(void)
 	mutex_destroy(&poweroff_lock);
 	return result;
 }
-core_initcall(thermal_init);
+postcore_initcall(thermal_init);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] thermal: core: Move initialization after core initcall
  2020-07-17 16:42 ` [PATCH 2/2] thermal: core: Move initialization after core initcall Daniel Lezcano
@ 2020-07-20  5:39   ` Amit Kucheria
  0 siblings, 0 replies; 4+ messages in thread
From: Amit Kucheria @ 2020-07-20  5:39 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: Zhang Rui, LKML, Linux PM list, Marek Szyprowski

On Fri, Jul 17, 2020 at 10:12 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> The generic netlink is initialized at subsys_initcall, so far after
> the thermal init routine and the thermal generic netlink family
> initialization.
>
> On ŝome platforms, that leads to a memory corruption.
>
> The fix was sent to netdev@ to move the genetlink framework
> initialization at core_initcall.
>
> Move the thermal core initialization to postcore level which is very
> close to core level.
>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/thermal/thermal_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index c2e7d7aaa354..79551bb6cd4c 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1617,4 +1617,4 @@ static int __init thermal_init(void)
>         mutex_destroy(&poweroff_lock);
>         return result;
>  }
> -core_initcall(thermal_init);
> +postcore_initcall(thermal_init);

For posterity, we moved to core_initcall from fs_initcall since we had
removed netlink support from the thermal framework and we wanted to
initialise it as quickly as possible after cpufreq to allow early
mitigation possibility.

Given the addition of the new netlink-based API to thermal framework,
I think moving to postcore_initcall is an acceptable compromise.

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] thermal: netlink: Improve the initcall ordering
  2020-07-17 16:42 [PATCH 1/2] thermal: netlink: Improve the initcall ordering Daniel Lezcano
  2020-07-17 16:42 ` [PATCH 2/2] thermal: core: Move initialization after core initcall Daniel Lezcano
@ 2020-07-20  5:43 ` Amit Kucheria
  1 sibling, 0 replies; 4+ messages in thread
From: Amit Kucheria @ 2020-07-20  5:43 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: Zhang Rui, LKML, Linux PM list, Marek Szyprowski

On Fri, Jul 17, 2020 at 10:12 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> The initcalls like to play joke. In our case, the thermal-netlink
> initcall is called after the thermal-core initcall but this one sends
> a notification before the former is initialzed. No issue was spotted,

typo: initialized

> but it could lead to a memory corruption, so instead of relying on the
> core_initcall for the thermal-netlink, let's initialize directly from
> the thermal-core init routine, so we have full control of the init
> ordering.

> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  drivers/thermal/thermal_core.c    | 4 ++++
>  drivers/thermal/thermal_netlink.c | 3 +--
>  drivers/thermal/thermal_netlink.h | 6 ++++++
>  3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 25ef29123f72..c2e7d7aaa354 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1581,6 +1581,10 @@ static int __init thermal_init(void)
>  {
>         int result;
>
> +       result = thermal_netlink_init();
> +       if (result)
> +               goto error;
> +
>         mutex_init(&poweroff_lock);
>         result = thermal_register_governors();
>         if (result)
> diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
> index dd0a3b889674..42eace7401da 100644
> --- a/drivers/thermal/thermal_netlink.c
> +++ b/drivers/thermal/thermal_netlink.c
> @@ -641,8 +641,7 @@ static struct genl_family thermal_gnl_family __ro_after_init = {
>         .n_mcgrps       = ARRAY_SIZE(thermal_genl_mcgrps),
>  };
>
> -static int __init thermal_netlink_init(void)
> +int __init thermal_netlink_init(void)
>  {
>         return genl_register_family(&thermal_gnl_family);
>  }
> -core_initcall(thermal_netlink_init);
> diff --git a/drivers/thermal/thermal_netlink.h b/drivers/thermal/thermal_netlink.h
> index 0ec28d105da5..828d1dddfa98 100644
> --- a/drivers/thermal/thermal_netlink.h
> +++ b/drivers/thermal/thermal_netlink.h
> @@ -6,6 +6,7 @@
>
>  /* Netlink notification function */
>  #ifdef CONFIG_THERMAL_NETLINK
> +int __init thermal_netlink_init(void);
>  int thermal_notify_tz_create(int tz_id, const char *name);
>  int thermal_notify_tz_delete(int tz_id);
>  int thermal_notify_tz_enable(int tz_id);
> @@ -23,6 +24,11 @@ int thermal_notify_cdev_delete(int cdev_id);
>  int thermal_notify_tz_gov_change(int tz_id, const char *name);
>  int thermal_genl_sampling_temp(int id, int temp);
>  #else
> +static inline int thermal_netlink_init(void)
> +{
> +       return 0;
> +}
> +
>  static inline int thermal_notify_tz_create(int tz_id, const char *name)
>  {
>         return 0;
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-20  5:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 16:42 [PATCH 1/2] thermal: netlink: Improve the initcall ordering Daniel Lezcano
2020-07-17 16:42 ` [PATCH 2/2] thermal: core: Move initialization after core initcall Daniel Lezcano
2020-07-20  5:39   ` Amit Kucheria
2020-07-20  5:43 ` [PATCH 1/2] thermal: netlink: Improve the initcall ordering Amit Kucheria

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).