From owner-svn-src-projects@freebsd.org Tue Sep 3 14:07:26 2019 Return-Path: Delivered-To: svn-src-projects@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 8C572DDA84 for ; Tue, 3 Sep 2019 14:07:25 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [96.47.72.132]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "freefall.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46N80X4SFNz4QZm; Tue, 3 Sep 2019 14:07:24 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 1452) id 9D02E1B5D6; Tue, 3 Sep 2019 14:06:38 +0000 (UTC) X-Original-To: yuripv@localmail.freebsd.org Delivered-To: yuripv@localmail.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by freefall.freebsd.org (Postfix) with ESMTPS id 7AFC176D2; Tue, 23 Apr 2019 16:48:22 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2610:1c1:1:6074::16:84]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "freefall.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 7EA8F81330; Tue, 23 Apr 2019 16:48:21 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 538) id 27F9D768E; Tue, 23 Apr 2019 16:48:21 +0000 (UTC) Delivered-To: src-committers@localmail.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by freefall.freebsd.org (Postfix) with ESMTPS id DEF3C7684 for ; Tue, 23 Apr 2019 16:48:17 +0000 (UTC) (envelope-from asomers@gmail.com) Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com [209.85.208.193]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4B04081320; Tue, 23 Apr 2019 16:48:17 +0000 (UTC) (envelope-from asomers@gmail.com) Received: by mail-lj1-f193.google.com with SMTP id p14so14150854ljg.5; Tue, 23 Apr 2019 09:48:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=i7phNkJ526gruixG/xR56CxNl5gmpFXnV/CosAH+P7E=; b=XUjJA0QSokPmBpIvCDvmnBBaDIO/tLSzWVkH1HgA9sgIhXFTxgHC14BI+DemSdFRTP SlCIWgjVRDC6Aa0aHysnT8j4GZ+eaA2MH95iBLR6s5C36/Z7eerQEY0uPiyMUwYR/kCX sQauUvqgnspCCSAJF5TA8zF6NbhXuTqxFmqfFJ+jVh3fC8ZBU/0VdOz+/9Fk86x8DJmB m88Q4679Kn9P1Rvhj2YKLriiKavCpOJHQab0rkUqCg+VhFsbhdkg7dum1nMU2z/FfWHr gqle/8Dpxn/+H9+WLRz3QiuoDr5sK2hDoyUJdoJ0j3dXxMghaQO3tvwYiK8wv2G/NCc3 IVAw== X-Gm-Message-State: APjAAAUizZMLqDuI5uw6SahkL9Y1t9yuVq7gt85SgmrRjAkZK/rgcWdJ 9Cz2WzWYy2wctqeD2/oVWRUZYgUKZMqMu2tIcL7s6ygG X-Google-Smtp-Source: APXvYqyr04q09Sw25QTCtv+gX1W7doPXJ9uss0xgFcBHpy7V9SNFMAudcShoSd24C6HEatEvBUNouCAwizg2KDSL8h4= X-Received: by 2002:a2e:9719:: with SMTP id r25mr12952577lji.29.1556038090092; Tue, 23 Apr 2019 09:48:10 -0700 (PDT) MIME-Version: 1.0 References: <201904212304.x3LN46Pt046728@repo.freebsd.org> <20190422171033.GX12936@kib.kiev.ua> <20190423124657.GY12936@kib.kiev.ua> <20190423160826.GZ12936@kib.kiev.ua> In-Reply-To: <20190423160826.GZ12936@kib.kiev.ua> From: Alan Somers Message-ID: Subject: Re: svn commit: r346507 - in projects/fuse2/sys: kern sys To: Konstantin Belousov Cc: src-committers , svn-src-projects@freebsd.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk X-Loop: FreeBSD.org Sender: owner-src-committers@freebsd.org X-Rspamd-Queue-Id: 7EA8F81330 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.95 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.95)[-0.952,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] Status: O X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.29 List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Date: Tue, 03 Sep 2019 14:07:26 -0000 X-Original-Date: Tue, 23 Apr 2019 10:47:58 -0600 X-List-Received-Date: Tue, 03 Sep 2019 14:07:26 -0000 On Tue, Apr 23, 2019 at 10:08 AM Konstantin Belousov wrote: > > On Tue, Apr 23, 2019 at 08:50:21AM -0600, Alan Somers wrote: > > On Tue, Apr 23, 2019 at 6:47 AM Konstantin Belousov wrote: > > > > > > On Mon, Apr 22, 2019 at 11:27:54AM -0600, Alan Somers wrote: > > > > On Mon, Apr 22, 2019 at 11:10 AM Konstantin Belousov > > > > wrote: > > > > > > > > > > On Sun, Apr 21, 2019 at 11:04:06PM +0000, Alan Somers wrote: > > > > > > Author: asomers > > > > > > Date: Sun Apr 21 23:04:06 2019 > > > > > > New Revision: 346507 > > > > > > URL: https://svnweb.freebsd.org/changeset/base/346507 > > > > > > > > > > > > Log: > > > > > > fusefs: commit missing files from r346387 > > > > > > > > > > > > PR: 346357 > > > > > > Sponsored by: The FreeBSD Foundation > > > > > > > > > > > > Modified: > > > > > > projects/fuse2/sys/kern/kern_sig.c > > > > > > projects/fuse2/sys/sys/signalvar.h > > > > > > > > > > > > Modified: projects/fuse2/sys/kern/kern_sig.c > > > > > > ============================================================================== > > > > > > --- projects/fuse2/sys/kern/kern_sig.c Sun Apr 21 22:53:51 2019 (r346506) > > > > > > +++ projects/fuse2/sys/kern/kern_sig.c Sun Apr 21 23:04:06 2019 (r346507) > > > > > > @@ -929,6 +929,23 @@ osigreturn(struct thread *td, struct osigreturn_args * > > > > > > #endif > > > > > > #endif /* COMPAT_43 */ > > > > > > > > > > > > +/* Will this signal be fatal to the current process ? */ > > > > > > +bool > > > > > > +sig_isfatal(struct proc *p, int sig) > > > > > > +{ > > > > > > + intptr_t act; > > > > > > + > > > > > > + act = (intptr_t)p->p_sigacts->ps_sigact[_SIG_IDX(sig)]; > > > > > > + if ((intptr_t)SIG_DFL == act) { > > > > > > + int prop; > > > > > This is against style. > > > > > > > > > > > + > > > > > > + prop = sigprop(sig); > > > > > > + return (0 != (prop & (SIGPROP_KILL | SIGPROP_CORE))); > > > > > > + } else { > > > > > > + return (false); > > > > > > + } > > > > > > +} > > > > > Either your function lacks asserts about the owned locks, or it is racy. > > > > > > > > Good point. I'll add lock assertions. > > > > > > > > > > > > > > Said that, is the comment above describes the intent ? The > > > > > implementation is too naive. Just for example, blocked signals with > > > > > default disposition do not result in the termination. On the other hand, > > > > > blocked ignored traps cause immediate termination. > > > > > > > > I'm using this in a context where the signal has already been > > > > delivered and caught. So it can't be blocked, and it can't be a trap. > > > > > > > > > > > > > > Overall, I do not believe that it is possible to implement that without > > > > > duplicating the code of tdsendsignal() and trapsignal(), i.e. you should > > > > > additionally provide the originating context, besides a signal number. > > > > > > > > Do you still believe that even though it doesn't need to consider > > > > blocked signals and traps? > > > Generally yes, but lets see the specifics of the use. > > > > > > > > > > > > > > > > > What you are trying to do there ? > > > > > > > > It's in a situation where a syscall can't simply return EINTR or > > > > ERESTART. I need to do some extra work to interrupt the syscall (ask > > > > the FUSE daemon to interrupt the associated FUSE operation). If the > > > > signal will be fatal, then there's no point in waiting for the FUSE > > > > daemon to reply and I can simply return EINTR. However, if the signal > > > > is not fatal, then I need to wait to see if the FUSE daemon to > > > > acknowledge the interrupt or else complete the operation like normal. > > > In what context does the interruption happen ? Is it for a thread of the > > > fuse daemon, or normal process ? > > > > It's usually a normal process, but it could be another kernel thread, > > like aiod. It will never be the fuse daemon. > > > > > > > > Can you point out the specific fragment of code where the function is used ? > > > > Here's the function that uses it: > > https://svnweb.freebsd.org/base/projects/fuse2/sys/fs/fuse/fuse_ipc.c?view=markup#l429 > > This is even more confusing. > > I do not understand the kern_sigprocmask() calls around msleep(9) with > empty SIG_BLOCK mask, this looks like a nop. What is the purpose ? The mask is only empty the first time through. If a signal is received, then it gets added to blockedset and the msleep is retried. > > Then, calling cursig() has much more implications than just returning > the lowest pending signal number, for instance it redirects the signal > to debugger, or reacts to SIGSTOP. Do you have any resources owned at > this time, most likely a vnode lock ? Probably. Is there a better way to get the pending signal number? > > What if another thread in the mt process terminates the process, and now > waits for this one to reach the safe boundary ? If this thread gets a fatal signal, then it will return ASAP. If it doesn't, then it will wait for the FUSE daemon to respond to the outstanding operation or for the operation to time out, which is what it always did prior to my recent commit. > > Overall, it is not a filesystem business to even look into the signal > state of the process. You should react to EINTR/ERESTART from msleep(9) > at most. > > NFS approach, although not very good, is definitely better than this. > NFS provides intr mount option vs. hard (default) mounts. If intr > option is specified, then VOPs sleep with PCATCH, and pass EINTR/ERESTART > up, orphaning the request. For hard mounts, PCATCH is not used. > > You would need to complete in-flight fuse daemon requests even if orphaned > by the requested thread due to the interruption. I think the NFS approach works because NFS is stateless and its operations are idempotent. But that's not true of FUSE. For example, if a process writing to tape via sysutils/ltfs gets signaled, it needs to know whether or not its last write operation completed, and when. > > There is very delicate TDF_SBDRY mechanism which defers stopping of the > sleeping threads which owns resources like vnode locks, and which controls > controls the reaction to the pending signal (selection of EINTR vs ERESTART, > which is important for advisory locks because they must not auto-restart). > It is used by NFS to avoid situation where the thread inside msleep(PCATCH) > is stopped by SIGSTOP, and then vnode lock cascade kills all filesystem > accesses. See sigdeferstop()/sigallowstop(). Thanks for the tip. I'll try those. I'll defer stops before calling cursig. Or do I need to defer stops whenever I might have a vnode lock? If so, then I should probably just block stop signals. -Alan