From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF9EFC77B75 for ; Tue, 16 May 2023 20:12:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230161AbjEPUM4 (ORCPT ); Tue, 16 May 2023 16:12:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60024 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229914AbjEPUMy (ORCPT ); Tue, 16 May 2023 16:12:54 -0400 Received: from out03.mta.xmission.com (out03.mta.xmission.com [166.70.13.233]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7AD65421A for ; Tue, 16 May 2023 13:12:52 -0700 (PDT) Received: from in01.mta.xmission.com ([166.70.13.51]:33060) by out03.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1pz12Q-002ZBM-Ml; Tue, 16 May 2023 14:12:50 -0600 Received: from ip68-110-29-46.om.om.cox.net ([68.110.29.46]:41298 helo=email.froward.int.ebiederm.org.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1pz12P-005t0v-G7; Tue, 16 May 2023 14:12:50 -0600 From: "Eric W. Biederman" To: Oleg Nesterov Cc: Linus Torvalds , Mike Christie , Christian Brauner , Thorsten Leemhuis , nicolas.dichtel@6wind.com, Linux kernel regressions list , hch@infradead.org, stefanha@redhat.com, jasowang@redhat.com, mst@redhat.com, sgarzare@redhat.com, virtualization@lists.linux-foundation.org, konrad.wilk@oracle.com, linux-kernel@vger.kernel.org, Jens Axboe References: <78c5e150-26cf-7724-74ee-4a0b16b944b1@oracle.com> <48842e92-835e-bc3f-7118-48b8f415f532@leemhuis.info> <20230515-vollrausch-liebgeworden-2765f3ca3540@brauner> <122b597e-a5fa-daf7-27bb-6f04fa98d496@oracle.com> <87cz30s20y.fsf@email.froward.int.ebiederm.org> <20230516183757.GA1286@redhat.com> Date: Tue, 16 May 2023 15:12:10 -0500 In-Reply-To: <20230516183757.GA1286@redhat.com> (Oleg Nesterov's message of "Tue, 16 May 2023 20:37:57 +0200") Message-ID: <87mt24ox2d.fsf@email.froward.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1pz12P-005t0v-G7;;;mid=<87mt24ox2d.fsf@email.froward.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.110.29.46;;;frm=ebiederm@xmission.com;;;spf=pass X-XM-AID: U2FsdGVkX1//rdFVubSWApRxlVrJbJVsmsZmIf/63yc= X-SA-Exim-Connect-IP: 68.110.29.46 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Oleg Nesterov writes: > On 05/16, Eric W. Biederman wrote: >> >> A kernel thread can block SIGKILL and that is supported. >> >> For a thread that is part of a process you can't block SIGKILL when the >> task is part of a user mode process. > > Or SIGSTOP. Another thread can call do_signal_stop()->signal_wake_up/etc. Yes, ignoring SIGSTOP leads to the same kind of rendezvous issues as SIGKILL. >> There is this bit in complete_signal when SIGKILL is delivered to any >> thread in the process. >> >> t = p; >> do { >> task_clear_jobctl_pending(t, JOBCTL_PENDING_MASK); >> sigaddset(&t->pending.signal, SIGKILL); >> signal_wake_up(t, 1); >> } while_each_thread(p, t); > > That is why the latest version adds try_set_pending_sigkill(). No, no, > it is not that I think this is a good idea. I see that try_set_pending_sigkill in the patch now. That try_set_pending_sigkill just keeps the process from reporting that it has exited, and extend the process exit indefinitely. SIGNAL_GROUP_EXIT has already been set, so the KILL signal was already delivered and the process is exiting. >> For clarity that sigaddset(&t->pending.signal, SIGKILL); Really isn't >> setting SIGKILL pending, > > Hmm. it does? Nevermind. The point is that what try_set_pending_sigkill in the patch is doing is keeping the "you are dead exit now" flag, from being set. That flag is what fatal_signal_pending always tests, because we can only know if a fatal signal is pending if we have performed short circuit delivery on the signal. The result is the effects of the change are mostly what people expect. The difference the semantics being changed aren't what people think they are. AKA process exit is being ignored for the thread, not that SIGKILL is being blocked. >> The important part of that code is that SIGNAL_GROUP_EXIT gets set. >> That indicates the entire process is being torn down. > > Yes. and the same is true for io-thread even if it calls get_signal() > and dequeues SIGKILL and clears TIF_SIGPENDING. > >> but in that case the vhost logic needs to act like a process, just >> like io_uring does. > > confused... create_io_thread() creates a sub-thread too? Yes, create_io_uring creates an ordinary user space thread that never runs any code in user space. > Although I never understood this logic. I can't even understand the usage > of lower_32_bits() in create_io_thread(). As far as I can tell lower_32_bits(flags) is just defensive programming that just copies the code in clone. The code just as easily have said u32 flags, or have just populated .flags directly. Then .exit_signal could have been set to 0. Later copy_process will set .exit_signal = -1 because CLONE_THREAD is set. The reason for adding create_io_thread calling copy_process as I recall so that the new task does not start automatically. This allows functions like io_init_new_worker to initialize the new task without races and then call wake_up_new_task. Eric