[PATCH 0/3] powerpc: Fixes and optimization for the freezable kthread

[PATCH 0/3] powerpc: Fixes and optimization for the freezable kthread

[Kevin Hao]

Hi,

The main changes include:
  - Invoke set_freezable() for the kthread which could be frozen
  - Drop redundant try_to_freeze() invocation

Kevin Hao (3):
  powerpc: mpc83xx: Add the missing set_freezable() for
    agent_thread_fn()
  powerpc: mpc83xx: Use wait_event_freezable() for freezable kthread
  powerpc: ps3: Add missing set_freezable() for ps3_probe_thread()

 arch/powerpc/platforms/83xx/suspend.c    | 5 +++--
 arch/powerpc/platforms/ps3/device-init.c | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.39.2

[PATCH 1/3] powerpc: mpc83xx: Add the missing set_freezable() for agent_thread_fn()

[Kevin Hao]

The kernel thread function agent_thread_fn() invokes the try_to_freeze()
in its loop. But all the kernel threads are non-freezable by default.
So if we want to make a kernel thread to be freezable, we have to invoke
set_freezable() explicitly.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/platforms/83xx/suspend.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index 9833c36bda83..eed325ed08cc 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -261,6 +261,8 @@ static int mpc83xx_suspend_begin(suspend_state_t state)
 
 static int agent_thread_fn(void *data)
 {
+	set_freezable();
+
 	while (1) {
 		wait_event_interruptible(agent_wq, pci_pm_state >= 2);
 		try_to_freeze();
-- 
2.39.2

[PATCH 2/3] powerpc: mpc83xx: Use wait_event_freezable() for freezable kthread

[Kevin Hao]

A freezable kernel thread can enter frozen state during freezing by
either calling try_to_freeze() or using wait_event_freezable() and its
variants. So for the following snippet of code in a kernel thread loop:
  wait_event_interruptible();
  try_to_freeze();

We can change it to a simple wait_event_freezable() and then eliminate
a function call.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/platforms/83xx/suspend.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index eed325ed08cc..c9664e46b03d 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -264,8 +264,7 @@ static int agent_thread_fn(void *data)
 	set_freezable();
 
 	while (1) {
-		wait_event_interruptible(agent_wq, pci_pm_state >= 2);
-		try_to_freeze();
+		wait_event_freezable(agent_wq, pci_pm_state >= 2);
 
 		if (signal_pending(current) || pci_pm_state < 2)
 			continue;
-- 
2.39.2

[PATCH 3/3] powerpc: ps3: Add missing set_freezable() for ps3_probe_thread()

[Kevin Hao]

The kernel thread function ps3_probe_thread() invokes the try_to_freeze()
in its loop. But all the kernel threads are non-freezable by default.
So if we want to make a kernel thread to be freezable, we have to invoke
set_freezable() explicitly.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/platforms/ps3/device-init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index e87360a0fb40..878bc160246e 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -827,6 +827,7 @@ static int ps3_probe_thread(void *data)
 	if (res)
 		goto fail_free_irq;
 
+	set_freezable();
 	/* Loop here processing the requested notification events. */
 	do {
 		try_to_freeze();
-- 
2.39.2

Re: [PATCH 3/3] powerpc: ps3: Add missing set_freezable() for ps3_probe_thread()

[Geoff Levand]

Hi Kevin,

On 12/21/23 13:45, Kevin Hao wrote:
> The kernel thread function ps3_probe_thread() invokes the try_to_freeze()
> in its loop. But all the kernel threads are non-freezable by default.
> So if we want to make a kernel thread to be freezable, we have to invoke
> set_freezable() explicitly.
> 
> Signed-off-by: Kevin Hao <haokexin@gmail.com>
> ---
>  arch/powerpc/platforms/ps3/device-init.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
> index e87360a0fb40..878bc160246e 100644
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -827,6 +827,7 @@ static int ps3_probe_thread(void *data)
>  	if (res)
>  		goto fail_free_irq;
>  
> +	set_freezable();
>  	/* Loop here processing the requested notification events. */
>  	do {
>  		try_to_freeze();

Seems like a reasonable addition.

Signed-off-by: Geoff Levand <geoff@infradead.org>

Re: [PATCH 3/3] powerpc: ps3: Add missing set_freezable() for ps3_probe_thread()

[Michael Ellerman]

Geoff Levand <geoff@infradead.org> writes:
> Hi Kevin,
>
> On 12/21/23 13:45, Kevin Hao wrote:
>> The kernel thread function ps3_probe_thread() invokes the try_to_freeze()
>> in its loop. But all the kernel threads are non-freezable by default.
>> So if we want to make a kernel thread to be freezable, we have to invoke
>> set_freezable() explicitly.
>> 
>> Signed-off-by: Kevin Hao <haokexin@gmail.com>
>> ---
>>  arch/powerpc/platforms/ps3/device-init.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
>> index e87360a0fb40..878bc160246e 100644
>> --- a/arch/powerpc/platforms/ps3/device-init.c
>> +++ b/arch/powerpc/platforms/ps3/device-init.c
>> @@ -827,6 +827,7 @@ static int ps3_probe_thread(void *data)
>>  	if (res)
>>  		goto fail_free_irq;
>>  
>> +	set_freezable();
>>  	/* Loop here processing the requested notification events. */
>>  	do {
>>  		try_to_freeze();
>
> Seems like a reasonable addition.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>

I turned that into an Acked-by, which I think is what you meant :)

cheers

Re: [PATCH 0/3] powerpc: Fixes and optimization for the freezable kthread

[Michael Ellerman]

On Thu, 21 Dec 2023 12:45:07 +0800, Kevin Hao wrote:
> The main changes include:
>   - Invoke set_freezable() for the kthread which could be frozen
>   - Drop redundant try_to_freeze() invocation
> 
> Kevin Hao (3):
>   powerpc: mpc83xx: Add the missing set_freezable() for
>     agent_thread_fn()
>   powerpc: mpc83xx: Use wait_event_freezable() for freezable kthread
>   powerpc: ps3: Add missing set_freezable() for ps3_probe_thread()
> 
> [...]

Applied to powerpc/next.

[1/3] powerpc: mpc83xx: Add the missing set_freezable() for agent_thread_fn()
      https://git.kernel.org/powerpc/c/6addc560e69cd1b2e68ef43ad62a878ac1956f51
[2/3] powerpc: mpc83xx: Use wait_event_freezable() for freezable kthread
      https://git.kernel.org/powerpc/c/11611d254c15cce1f58431b2965c6edb5aa7e610
[3/3] powerpc: ps3: Add missing set_freezable() for ps3_probe_thread()
      https://git.kernel.org/powerpc/c/ccc0f7b7673e63139ba9d916f4567d4fadb14b55

cheers