From owner-svn-src-all@freebsd.org Tue Sep 3 14:07:29 2019 Return-Path: Delivered-To: svn-src-all@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 86CB6DCE17; Tue, 3 Sep 2019 14:06:42 +0000 (UTC) (envelope-from yuripv@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 46N7zj6NZ6z4PpP; Tue, 3 Sep 2019 14:06:41 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 1452) id 8FF541AA3C; Tue, 3 Sep 2019 14:06:14 +0000 (UTC) X-Original-To: yuripv@localmail.freebsd.org Delivered-To: yuripv@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 0C6636E4C; Thu, 11 Apr 2019 16:26:31 +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 46ABB6D9C1; Thu, 11 Apr 2019 16:26:30 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 538) id 18AAF6E03; Thu, 11 Apr 2019 16:26:30 +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 D56F86DFE; Thu, 11 Apr 2019 16:26:27 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-it1-f173.google.com (mail-it1-f173.google.com [209.85.166.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 993A46D9B7; Thu, 11 Apr 2019 16:26:27 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-it1-f173.google.com with SMTP id k64so10507354itb.5; Thu, 11 Apr 2019 09:26:27 -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:reply-to :from:date:message-id:subject:to:cc; bh=FjZy8WxMn4IV88mdYknSv4QPvQ4miQE75i20Vq3xxJc=; b=Brs8STVo+5+iEWUhrvKP19psROkTEjs/Hy2h9nKAufATlje8aMcpp5n/vPj/U3BIjq c3zNyuIkhKnMYROX5UwYZh9IPTME7cH2Sn8sDMB2g3TnuL8hWV/aX4CbQKHAZMWLGyF0 TSJ2jp2rRlYokfINjuWIfZv8j0aU18JCYX/16qb7g4t/aVDzgQAEyN258mPt/aZm1j+w UpBHU09NS2mlAlR+0056dBlC/UONB6A7EqBTVluQA8SsdiAld6vi2qDpyJWK0H53w2zv 0NSyVcXSL7pewV1KrF3RZiqaLu17wJ7qYbUnW66F7CTUahNxdSTKqMuMgUZOtjJFa9EN 2AFg== X-Gm-Message-State: APjAAAUGkTm+9bbu1TyvP0g78jYUsqU1/ZVSzFeiu13wNhOnLQyNk6CS Oiohjhv6fzygdApiNSkLTpifGWDd X-Google-Smtp-Source: APXvYqxsCUEhpKK2GiPhHLW8YEO069iXKOEK3WWNrF2L9Dd0lp/m8A4TCd3eeyZCNQptkFFRTf+Lmg== X-Received: by 2002:a02:212c:: with SMTP id e44mr35917840jaa.5.1554998366890; Thu, 11 Apr 2019 08:59:26 -0700 (PDT) Received: from mail-it1-f178.google.com (mail-it1-f178.google.com. [209.85.166.178]) by smtp.gmail.com with ESMTPSA id e26sm14970318iom.56.2019.04.11.08.59.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Apr 2019 08:59:26 -0700 (PDT) Received: by mail-it1-f178.google.com with SMTP id k64so10349641itb.5; Thu, 11 Apr 2019 08:59:26 -0700 (PDT) X-Received: by 2002:a24:4584:: with SMTP id c4mr9118943itd.163.1554998365847; Thu, 11 Apr 2019 08:59:25 -0700 (PDT) MIME-Version: 1.0 References: <201904111121.x3BBLj2K023087@repo.freebsd.org> In-Reply-To: <201904111121.x3BBLj2K023087@repo.freebsd.org> Reply-To: cem@freebsd.org From: Conrad Meyer X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r346120 - head/sys/kern To: Edward Tomasz Napierala Cc: src-committers , svn-src-all , svn-src-head Content-Type: text/plain; charset="UTF-8" Precedence: bulk X-Loop: FreeBSD.org Sender: owner-src-committers@freebsd.org X-Rspamd-Queue-Id: 46ABB6D9C1 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.97 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.98)[-0.975,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] Status: O X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Date: Tue, 03 Sep 2019 14:07:29 -0000 X-Original-Date: Thu, 11 Apr 2019 08:59:14 -0700 X-List-Received-Date: Tue, 03 Sep 2019 14:07:29 -0000 Hi Edward, I have a question about this change below. On Thu, Apr 11, 2019 at 4:22 AM Edward Tomasz Napierala wrote: > > Author: trasz > Date: Thu Apr 11 11:21:45 2019 > New Revision: 346120 > URL: https://svnweb.freebsd.org/changeset/base/346120 > > Log: > Use shared vnode locks for the ELF interpreter. > > ... > Differential Revision: https://reviews.freebsd.org/D19874 > ... > Modified: head/sys/kern/imgact_elf.c > ============================================================================== > --- head/sys/kern/imgact_elf.c Thu Apr 11 08:06:45 2019 (r346119) > +++ head/sys/kern/imgact_elf.c Thu Apr 11 11:21:45 2019 (r346120) > ... > - NDINIT(nd, LOOKUP, LOCKLEAF | FOLLOW, UIO_SYSSPACE, file, curthread); > + flags = FOLLOW | LOCKSHARED | LOCKLEAF; > + > +again: > + NDINIT(nd, LOOKUP, flags, UIO_SYSSPACE, file, curthread); > if ((error = namei(nd)) != 0) { > ... > @@ -759,15 +762,30 @@ __elfN(load_file)(struct proc *p, const char *file, u_ > ... > + if (VOP_IS_TEXT(nd->ni_vp) == 0) { > + if (VOP_ISLOCKED(nd->ni_vp) != LK_EXCLUSIVE) { > + /* > + * LK_UPGRADE could have resulted in dropping > + * the lock. Just try again from the start, > + * this time with exclusive vnode lock. > + */ > + vput(nd->ni_vp); > + flags &= ~LOCKSHARED; > + goto again; It's unclear to me why we don't attempt LK_UPGRADE first. If upgrade succeeds, we avoid an extra filesystem traversal (namei/lookup). If it fails, of course we can 'goto again' the same as we do unconditionally here. There was some discussion about the topic in the linked phabricator PR with Konstantin, but I did not follow it fully. On the one hand, perhaps VOP_IS_TEXT() is rarely false for common interpreters anyway. On the other hand, there is sort of a renaissance of static linking happening. So maybe the thought is, !VOP_IS_TEXT is likely to be rare, and LK_UPGRADE success even more rare, so why bother writing additional code for it? Thanks, Conrad P.S., It is orthogonal to this discussion, but I don't see any reason for VOP_IS_TEXT to be a vnode_if operation. Neither it, nor VOP_UNSET_TEXT, is ever specialized. They simply check or clear the VV_TEXT flag on the vnode's vflags, respectively. It is common for the kernel to reach out and interact with other vnode vflags directly; e.g., pretty much all other VV_flags, like VV_ROOT. The only specialization of VOP_SET_TEXT is NFSclient, and it is unclear to me why the same requirements NFS client has for setting VV_TEXT do not apply universally.