freezer: allow killing of frozen tasks

Change __refrigerator() to allow SIGKILL signal handling during the frozen state (by setting task to a TASK_KILLABLE state instead of TASK_UNINTERRUPTIBLE one before entering sleep) and make tasks leave __refrigerator() upon receiving such signal.

These changes allow frozen tasks to be killed immediately without the need to thaw them first.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 kernel/freezer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

kernel/freezer.c +3 -2

index b462fa1..0c05a7a 100644
@@ -57,12 +57,13 @@ bool __refrigerator(bool check_kthr_stop)
 	pr_debug("%s entered refrigerator\n", current->comm);
 
 	for (;;) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
+		set_current_state(TASK_KILLABLE);
 
 		spin_lock_irq(&freezer_lock);
 		current->flags |= PF_FROZEN;
 		if (!freezing(current) ||
-		    (check_kthr_stop && kthread_should_stop()))
+		    (check_kthr_stop && kthread_should_stop()) ||
+		    fatal_signal_pending(current))
 			current->flags &= ~PF_FROZEN;
 		spin_unlock_irq(&freezer_lock);
 

Quoted 5 lines · Bartlomiej Zolnierkiewicz

Change __refrigerator() to allow SIGKILL signal handling during the frozen state (by setting task to a TASK_KILLABLE state instead of TASK_UNINTERRUPTIBLE one before entering sleep) and make tasks leave __refrigerator() upon receiving such signal.

Quoted 5 lines

These changes allow frozen tasks to be killed immediately without the need to thaw them first.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

Well, it doesn't sound like an entirely bad idea to me, but I'd like to know what Colin and Tejun (CCed now) think about it.

Hello,

Quoted 6 lines · Rafael J. Wysocki

Quoted 5 lines · Bartlomiej Zolnierkiewicz

Change __refrigerator() to allow SIGKILL signal handling during the frozen state (by setting task to a TASK_KILLABLE state instead of TASK_UNINTERRUPTIBLE one before entering sleep) and make tasks leave __refrigerator() upon receiving such signal.

Quoted 8 lines

Quoted 5 lines

These changes allow frozen tasks to be killed immediately without the need to thaw them first.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

Well, it doesn't sound like an entirely bad idea to me, but I'd like to know what Colin and Tejun (CCed now) think about it.

The problem is that we really don't know where each task is frozen in the kernel so don't know what happens after the task leaves the freezer is safe whether it's dying or not. We don't have any rules restricting where a freeze point should be and a task may do any operation between freezer and actual exit.

So, I don't think we can simply turn TASK_UNITERRUPTIBLE to TASK_KILLABLE at this point. We really need to strictly define where a task can freeze before being able to do anything like this.

Quoted 3 lines

So, I don't think we can simply turn TASK_UNITERRUPTIBLE to TASK_KILLABLE at this point. We really need to strictly define where a task can freeze before being able to do anything like this.

But we could do that for user space tasks I suppose?

Quoted 4 lines · Rafael J. Wysocki

Quoted 3 lines

So, I don't think we can simply turn TASK_UNITERRUPTIBLE to TASK_KILLABLE at this point. We really need to strictly define where a task can freeze before being able to do anything like this.

Quoted 1 line

But we could do that for user space tasks I suppose?

Even for userland tasks, we don't know where the task is stuck at. I think there are enough freeze points in the kernel which are in the middle of something which can be used by userland tasks excuting some syscall. We need to collect all those sites into well defined trap points before doing this.

Quoted 7 lines · Tejun Heo

Quoted 5 lines · Rafael J. Wysocki

Quoted 3 lines

So, I don't think we can simply turn TASK_UNITERRUPTIBLE to TASK_KILLABLE at this point. We really need to strictly define where a task can freeze before being able to do anything like this.

But we could do that for user space tasks I suppose?

Quoted 5 lines

Even for userland tasks, we don't know where the task is stuck at. I think there are enough freeze points in the kernel which are in the middle of something which can be used by userland tasks excuting some syscall. We need to collect all those sites into well defined trap points before doing this.

OK, thanks!

Quoted 14 lines · Rafael J. Wysocki

Quoted 12 lines · Tejun Heo

Quoted 5 lines · Rafael J. Wysocki

Quoted 3 lines

So, I don't think we can simply turn TASK_UNITERRUPTIBLE to TASK_KILLABLE at this point. We really need to strictly define where a task can freeze before being able to do anything like this.

But we could do that for user space tasks I suppose?

Even for userland tasks, we don't know where the task is stuck at. I think there are enough freeze points in the kernel which are in the middle of something which can be used by userland tasks excuting some syscall. We need to collect all those sites into well defined trap points before doing this.

Quoted 1 line

OK, thanks!

I scanned through try_to_freeze() users and it seems like we don't have that many which can be hit by userland tasks. I think it should be doable to audit all the users, remove the ones which can be invoked by userland and make try_to_freeze() whine loudly if it's running off a userland task except from well-defined spots.

Anyways, we need to ensure that userland task doesn't get stuck deep in the kernel before allowing this.

Quoted 15 lines · Tejun Heo

Quoted 14 lines · Rafael J. Wysocki

Quoted 12 lines · Tejun Heo

Quoted 5 lines · Rafael J. Wysocki

Quoted 3 lines

So, I don't think we can simply turn TASK_UNITERRUPTIBLE to TASK_KILLABLE at this point. We really need to strictly define where a task can freeze before being able to do anything like this.

But we could do that for user space tasks I suppose?

Even for userland tasks, we don't know where the task is stuck at. I think there are enough freeze points in the kernel which are in the middle of something which can be used by userland tasks excuting some syscall. We need to collect all those sites into well defined trap points before doing this.

Quoted 7 lines

Quoted 1 line

OK, thanks!

I scanned through try_to_freeze() users and it seems like we don't have that many which can be hit by userland tasks. I think it should be doable to audit all the users, remove the ones which can be invoked by userland and make try_to_freeze() whine loudly if it's running off a userland task except from well-defined spots.

Which might be worth doing anyway to be sure we know what's going on.

Quoted 2 lines

Anyways, we need to ensure that userland task doesn't get stuck deep in the kernel before allowing this.

Agreed.

Quoted 26 lines · Rafael J. Wysocki

Quoted 15 lines · Tejun Heo

Quoted 14 lines · Rafael J. Wysocki

Quoted 12 lines · Tejun Heo

Quoted 5 lines · Rafael J. Wysocki

Quoted 3 lines

So, I don't think we can simply turn TASK_UNITERRUPTIBLE to TASK_KILLABLE at this point. We really need to strictly define where a task can freeze before being able to do anything like this.

But we could do that for user space tasks I suppose?

Even for userland tasks, we don't know where the task is stuck at. I think there are enough freeze points in the kernel which are in the middle of something which can be used by userland tasks excuting some syscall. We need to collect all those sites into well defined trap points before doing this.

Quoted 7 lines

Quoted 1 line

OK, thanks!

I scanned through try_to_freeze() users and it seems like we don't have that many which can be hit by userland tasks. I think it should be doable to audit all the users, remove the ones which can be invoked by userland and make try_to_freeze() whine loudly if it's running off a userland task except from well-defined spots.

Which might be worth doing anyway to be sure we know what's going on.

Quoted 4 lines

Quoted 2 lines

Anyways, we need to ensure that userland task doesn't get stuck deep in the kernel before allowing this.

Agreed.

Are there any update? we need this feature to kill frozen app easily. Don't need to thaw app to kill.

Quoted 30 lines · Kyungmin Park

Quoted 29 lines · Rafael J. Wysocki

Quoted 15 lines · Tejun Heo

Quoted 14 lines · Rafael J. Wysocki

Quoted 12 lines · Tejun Heo

Quoted 5 lines · Rafael J. Wysocki

Quoted 3 lines

So, I don't think we can simply turn TASK_UNITERRUPTIBLE to TASK_KILLABLE at this point. We really need to strictly define where a task can freeze before being able to do anything like this.

But we could do that for user space tasks I suppose?

Even for userland tasks, we don't know where the task is stuck at. I think there are enough freeze points in the kernel which are in the middle of something which can be used by userland tasks excuting some syscall. We need to collect all those sites into well defined trap points before doing this.

Quoted 7 lines

Quoted 1 line

OK, thanks!

I scanned through try_to_freeze() users and it seems like we don't have that many which can be hit by userland tasks. I think it should be doable to audit all the users, remove the ones which can be invoked by userland and make try_to_freeze() whine loudly if it's running off a userland task except from well-defined spots.

Which might be worth doing anyway to be sure we know what's going on.

Quoted 2 lines

Anyways, we need to ensure that userland task doesn't get stuck deep in the kernel before allowing this.

Quoted 4 lines

Quoted 1 line

Agreed.

Are there any update? we need this feature to kill frozen app easily. Don't need to thaw app to kill.

No updates, but the above pretty much describes what needs to be done.