[RH7] ve: fix reading container state from cgroup

Submitted by Pavel Tikhomirov on March 30, 2018, 12:07 p.m.

Details

Message ID 20180330120727.26610-1-ptikhomirov@virtuozzo.com
State New
Series "ve: fix reading container state from cgroup"
Headers show

Commit Message

Pavel Tikhomirov March 30, 2018, 12:07 p.m.
1) ve->op_sem is needed to remove race with ve_start_container when
ve->is_running == 0 but ve->ve_ns != NULL already, and for a short
moment we would print "STOPPING" if read state on container start

2) replace !ve->init_task to !ve->ve_ns for simplicity they are changed
synchronously under ve->op_sem anyway, so lets do all checks with ve_ns

3) bring back !nr_threads_ve(ve) check as it is the only way we can
discern "STOPPED" from "STARTING"

So now we have:

RUNNING                 | STOPPED
is_running==true        | is_running==false
                        | ve_ns==NULL
                        | nr_threads_ve==0
------------------------|-----------------------
STOPPING                | STARTING
is_running==false       | is_running==false
ve_ns!=NULL             | ve_ns==NULL
                        | nr_threads_ve>0

Before these patch we were able to see STARTING instead of STOPPING and
vice versa.

Note: bug started to reproduce after "ve: fix container stopped state
check" part of commit 677f0715bb21 ("ve: initial patch").

https://jira.sw.ru/browse/PSBM-82766

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 kernel/ve/ve.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index f562fb9e5cbf..88e1c4cc3700 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -858,15 +858,17 @@  static int ve_state_read(struct cgroup *cg, struct cftype *cft,
 {
 	struct ve_struct *ve = cgroup_ve(cg);
 
+	down_read(&ve->op_sem);
 	if (ve->is_running)
 		seq_puts(m, "RUNNING");
-	else if (!ve->init_task)
+	else if (!nr_threads_ve(ve) && !ve->ve_ns)
 		seq_puts(m, "STOPPED");
 	else if (ve->ve_ns)
 		seq_puts(m, "STOPPING");
 	else
 		seq_puts(m, "STARTING");
 	seq_putc(m, '\n');
+	up_read(&ve->op_sem);
 
 	return 0;
 }

Comments

Kirill Tkhai March 30, 2018, 12:56 p.m.
On 30.03.2018 15:07, Pavel Tikhomirov wrote:
> 1) ve->op_sem is needed to remove race with ve_start_container when
> ve->is_running == 0 but ve->ve_ns != NULL already, and for a short
> moment we would print "STOPPING" if read state on container start
> 
> 2) replace !ve->init_task to !ve->ve_ns for simplicity they are changed
> synchronously under ve->op_sem anyway, so lets do all checks with ve_ns
> 
> 3) bring back !nr_threads_ve(ve) check as it is the only way we can
> discern "STOPPED" from "STARTING"
> 
> So now we have:
> 
> RUNNING                 | STOPPED
> is_running==true        | is_running==false
>                         | ve_ns==NULL
>                         | nr_threads_ve==0
> ------------------------|-----------------------
> STOPPING                | STARTING
> is_running==false       | is_running==false
> ve_ns!=NULL             | ve_ns==NULL
>                         | nr_threads_ve>0
> 
> Before these patch we were able to see STARTING instead of STOPPING and
> vice versa.
> 
> Note: bug started to reproduce after "ve: fix container stopped state
> check" part of commit 677f0715bb21 ("ve: initial patch").
> 
> https://jira.sw.ru/browse/PSBM-82766
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>

> ---
>  kernel/ve/ve.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index f562fb9e5cbf..88e1c4cc3700 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -858,15 +858,17 @@ static int ve_state_read(struct cgroup *cg, struct cftype *cft,
>  {
>  	struct ve_struct *ve = cgroup_ve(cg);
>  
> +	down_read(&ve->op_sem);
>  	if (ve->is_running)
>  		seq_puts(m, "RUNNING");
> -	else if (!ve->init_task)
> +	else if (!nr_threads_ve(ve) && !ve->ve_ns)
>  		seq_puts(m, "STOPPED");
>  	else if (ve->ve_ns)
>  		seq_puts(m, "STOPPING");
>  	else
>  		seq_puts(m, "STARTING");
>  	seq_putc(m, '\n');
> +	up_read(&ve->op_sem);
>  
>  	return 0;
>  }
>