Re: [PATCH] cgroup: freezer: don't change task and cgroups status unnecessarily

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 21, 2019 at 04:18:26PM +0800, Honglei Wang wrote:
> Seems it's not necessary to adjust the task state and revisit the
> state of source and destination cgroups if the cgroups are not in
> freeze state and the task itself is not frozen.
> 
> Signed-off-by: Honglei Wang <honglei.wang@xxxxxxxxxx>

Hello Honglei!

Overall the patch looks correct to me, but what's the goal of this change?
Do you see any performance difference in some tests?
Maybe you can use freezer tests from kselftests to show the difference?

Your patch will sightly change the behavior of the freezer if unfreezing
of a cgroup is racing with the task migration. The change is probably fine
(at least I can't imagine why not), and I'd totally support the patch,
if there is any performance benefit.

Thank you!

> ---
>  kernel/cgroup/freezer.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/kernel/cgroup/freezer.c b/kernel/cgroup/freezer.c
> index 8cf010680678..2dd66551d9a6 100644
> --- a/kernel/cgroup/freezer.c
> +++ b/kernel/cgroup/freezer.c
> @@ -230,6 +230,15 @@ void cgroup_freezer_migrate_task(struct task_struct *task,
>  	if (task->flags & PF_KTHREAD)
>  		return;
>  
> +	/*
> +	 * It's not necessary to do changes if both of the src and dst cgroups
> +	 * are not freeze and task is not frozen.
                       ^^^
		are not freezing?
> +	 */
> +	if (!test_bit(CGRP_FREEZE, &src->flags) &&
> +	    !test_bit(CGRP_FREEZE, &dst->flags) &&
> +	    !task->frozen)
> +		return;
> +
>  	/*
>  	 * Adjust counters of freezing and frozen tasks.
>  	 * Note, that if the task is frozen, but the destination cgroup is not
> -- 
> 2.17.0
> 




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux