Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1616984846-184466-1-git-send-email-zhaoqianligood@gmail.com>
In-Reply-To: <1616984846-184466-1-git-send-email-zhaoqianligood@gmail.com>
From:   qianli zhao <zhaoqianligood@gmail.com>
Date:   Tue, 30 Mar 2021 10:26:41 +0800
Message-ID: <CAPx_LQGMQzq=hQ0PzR_mXzi9Gp2X=5+Sk7fjvU5+QO_VVbnJQg@mail.gmail.com>
Subject: Re: [PATCH V4] exit: trigger panic when global init has exited
To:     christian@brauner.io, axboe@kernel.dk,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        Oleg Nesterov <oleg@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Collingbourne <pcc@google.com>
Cc:     linux-kernel@vger.kernel.org, Qianli Zhao <zhaoqianli@xiaomi.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

 Hi, Eric, Oleg

Any comment?

From the previous discussions, i think this change is necessary, but
we need to confirm that move the decrement of signal->live is a
safe.Here are some of my considerations
There are three places that are going to be called besides do_exit().
1. current_is_single_threaded()
current_is_single_threaded() is used to check current process just has
a single thread,my patch just moved the "signal->live--" position,this
won't change anything,current_is_single_threaded() maybe get different
value, after my patch,there is no change from the current logic.

2.css_task_iter_advance()
Same as above,css_task_iter_advance() just read "signal->live",this
may return different value,but it same before my patch.
css_task_iter_advance() cgroup_threadgroup_change_begin() held around
setting PF_EXITING before signal->live is decremented,
cgroup_threadgroup_rwsem(cgroup_threadgroup_change_begin()) is used
for user to get expect stable threadgroup,cgroup has no dependencies
on setting PF_EXITING or signal->live decrement.

3.copy_process()
copy_process() is called by fork(),copy_process will incremental
"signal->live",signal->live is atomic operation,there is no race, the
patch only move position,i don't see any new dependency problems

Moving the decrement position mainly changes the order in which
variables are assigned,we need to check if the change in the order of
assignment has any side effects on other callers.
i think acct_update_integrals(),sync_mm_rss() mainly updated some
data,only exit_signals() and sched_exit() need attention.
cgroup_threadgroup_change_begin() is called in exit_signals(),and
css_task_iter_advance used "signal->live",it seems like it might be a
little related.
cgroup_threadgroup_change_begin() just give stable threadgroup for
user,and css_task_iter_advance only check if group is dead, decrement
of signal->live and sets PF_EXITING seems like safe.

From my current analysis, this is safe.