From nobody Wed Sep 07 09:19:39 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4MMxY11grFz4c1Kk; Wed, 7 Sep 2022 09:19:49 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4MMxXz2Qy4z4H3V; Wed, 7 Sep 2022 09:19:47 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.16.1/8.16.1) with ESMTPS id 2879JdDc054392 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Wed, 7 Sep 2022 12:19:42 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by tom.home (8.16.1/8.16.1/Submit) id 2879JdWi054391; Wed, 7 Sep 2022 12:19:39 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 7 Sep 2022 12:19:39 +0300 From: Konstantin Belousov To: Mateusz Guzik Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 69413598d266 - main - signal: use proc_iterate to save on work Message-ID: References: <202209051156.285BuFWK076782@gitrepo.freebsd.org> List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on tom.home X-Rspamd-Queue-Id: 4MMxXz2Qy4z4H3V X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=fail reason="No valid SPF, No valid DKIM" header.from=gmail.com (policy=none); spf=softfail (mx1.freebsd.org: 2001:470:d5e7:1::1 is neither permitted nor denied by domain of kostikbel@gmail.com) smtp.mailfrom=kostikbel@gmail.com X-Spamd-Result: default: False [-2.45 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.45)[-0.452]; DMARC_POLICY_SOFTFAIL(0.10)[gmail.com : No valid SPF, No valid DKIM,none]; MIME_GOOD(-0.10)[text/plain]; FROM_EQ_ENVFROM(0.00)[]; FREEMAIL_TO(0.00)[gmail.com]; RCVD_TLS_LAST(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; MLMMJ_DEST(0.00)[dev-commits-src-all@freebsd.org,dev-commits-src-main@freebsd.org]; ASN(0.00)[asn:6939, ipnet:2001:470::/32, country:US]; TO_DN_SOME(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; RCPT_COUNT_THREE(0.00)[4]; FROM_HAS_DN(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; R_SPF_SOFTFAIL(0.00)[~all:c]; HAS_XAW(0.00)[]; RCVD_COUNT_THREE(0.00)[3]; ARC_NA(0.00)[] X-ThisMailContainsUnwantedMimeParts: N On Mon, Sep 05, 2022 at 06:07:08PM +0200, Mateusz Guzik wrote: > On 9/5/22, Konstantin Belousov wrote: > > On Mon, Sep 05, 2022 at 11:56:15AM +0000, Mateusz Guzik wrote: > >> The branch main has been updated by mjg: > >> > >> URL: > >> https://cgit.FreeBSD.org/src/commit/?id=69413598d2660054e29cac9454fe18c08e3bf36d > >> > >> commit 69413598d2660054e29cac9454fe18c08e3bf36d > >> Author: Mateusz Guzik > >> AuthorDate: 2022-03-10 18:58:12 +0000 > >> Commit: Mateusz Guzik > >> CommitDate: 2022-09-05 11:54:47 +0000 > >> > >> signal: use proc_iterate to save on work > >> > >> Most notably poudriere performs kill -9 -1 in jails for each port > >> being built. This reduces the scan from hundrends of processes to > >> literally 1. > >> > >> Reviewed by: jamie, markj > >> Differential Revision: https://reviews.freebsd.org/D34522 > >> --- > >> sys/kern/kern_sig.c | 39 ++++++++++++++++++++++++++++----------- > >> 1 file changed, 28 insertions(+), 11 deletions(-) > >> > >> diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c > >> index 6fd3eca0a14e..c50a37de07e6 100644 > >> --- a/sys/kern/kern_sig.c > >> +++ b/sys/kern/kern_sig.c > >> @@ -1776,18 +1776,13 @@ struct killpg1_ctx { > >> }; > >> > >> static void > >> -killpg1_sendsig(struct proc *p, bool notself, struct killpg1_ctx *arg) > >> +killpg1_sendsig_locked(struct proc *p, struct killpg1_ctx *arg) > >> { > >> int err; > >> > >> - if (p->p_pid <= 1 || (p->p_flag & P_SYSTEM) != 0 || > >> - (notself && p == arg->td->td_proc) || p->p_state == PRS_NEW) > >> - return; > >> - PROC_LOCK(p); > >> err = p_cansignal(arg->td, p, arg->sig); > >> if (err == 0 && arg->sig != 0) > >> pksignal(p, arg->sig, arg->ksi); > >> - PROC_UNLOCK(p); > >> if (err != ESRCH) > >> arg->found = true; > >> if (err == 0) > >> @@ -1796,6 +1791,31 @@ killpg1_sendsig(struct proc *p, bool notself, > >> struct killpg1_ctx *arg) > >> arg->ret = err; > >> } > >> > >> +static void > >> +killpg1_sendsig(struct proc *p, bool notself, struct killpg1_ctx *arg) > >> +{ > >> + > >> + if (p->p_pid <= 1 || (p->p_flag & P_SYSTEM) != 0 || > >> + (notself && p == arg->td->td_proc) || p->p_state == PRS_NEW) > >> + return; > >> + > >> + PROC_LOCK(p); > >> + killpg1_sendsig_locked(p, arg); > >> + PROC_UNLOCK(p); > >> +} > >> + > >> +static void > >> +kill_processes_prison_cb(struct proc *p, void *arg) > >> +{ > >> + struct killpg1_ctx *ctx = arg; > >> + > >> + if (p->p_pid <= 1 || (p->p_flag & P_SYSTEM) != 0 || > >> + (p == ctx->td->td_proc) || p->p_state == PRS_NEW) > > Extra () > > > >> + return; > >> + > >> + killpg1_sendsig_locked(p, ctx); > >> +} > >> + > >> /* > >> * Common code for kill process group/broadcast kill. > >> * cp is calling process. > >> @@ -1817,11 +1837,8 @@ killpg1(struct thread *td, int sig, int pgid, int > >> all, ksiginfo_t *ksi) > >> /* > >> * broadcast > >> */ > >> - sx_slock(&allproc_lock); > >> - FOREACH_PROC_IN_SYSTEM(p) { > >> - killpg1_sendsig(p, true, &arg); > >> - } > >> - sx_sunlock(&allproc_lock); > >> + prison_proc_iterate(td->td_ucred->cr_prison, > >> + kill_processes_prison_cb, &arg); > >> } else { > >> sx_slock(&proctree_lock); > >> if (pgid == 0) { > > > > I believe before your change, kill(-1) would kill all processes in the > > jail, which includes all processes in the nested jails. Now, it seems > > that linkage prevents iterating over the nested jails, am I missing it? > > > > See the pr_childcount check. If any jails pop up, there is a full scan > like right now. And if the count transitions 0 -> 1 -> 0 during the > iteration of the loop, there is nobody left to signal. I see. > > All previously existing races remain unaffected. No, now you are potentially signalling some processes more than once. Sending the signal is not idempotent operation.