Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 19 May 2019 01:09:40 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Toomas Soome <tsoome@me.com>
Cc:        Alexey Dokuchaev <danfe@freebsd.org>, Allan Jude <allanjude@freebsd.org>,  src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r347953 - head/cddl/contrib/opensolaris/cmd/zfs
Message-ID:  <20190519002125.B1574@besplex.bde.org>
In-Reply-To: <826320C2-9DF0-4C22-8234-E0F66F8AAEB2@me.com>
References:  <201905181227.x4ICRMsG073717@repo.freebsd.org> <20190518123708.GA73234@FreeBSD.org> <826320C2-9DF0-4C22-8234-E0F66F8AAEB2@me.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 18 May 2019, Toomas Soome wrote:

>> On 18 May 2019, at 15:37, Alexey Dokuchaev <danfe@freebsd.org> wrote:
>>
>> On Sat, May 18, 2019 at 12:27:22PM +0000, Allan Jude wrote:
>>> New Revision: 347953
>>> URL: https://svnweb.freebsd.org/changeset/base/347953
>>>
>>> Log:
>>>  MFV/ZoL: `zfs userspace` ignored all unresolved UIDs after the first
>>>
>>>  zfsonlinux/zfs@88cfff182432e4d1c24c877f33b47ee6cf109eee
>>>
>>>  zfs_main: fix `zfs userspace` squashing unresolved entries
>>>
>>> ...
>>> @@ -2368,10 +2369,12 @@ us_compare(const void *larg, const void *rarg, =
void *u
>>> =09=09=09=09if (rv64 !=3D lv64)
>>> =09=09=09=09=09rc =3D (rv64 < lv64) ? 1 : -1;
>>> =09=09=09} else {
>>> -=09=09=09=09(void) nvlist_lookup_string(lnvl, propname,
>>> -=09=09=09=09    &lvstr);
>>> -=09=09=09=09(void) nvlist_lookup_string(rnvl, propname,
>>> -=09=09=09=09    &rvstr);
>>> +=09=09=09=09if ((nvlist_lookup_string(lnvl, propname,
>>> +=09=09=09=09=09=09&lvstr) =3D=3D ENOENT) ||
>>> +=09=09=09=09    (nvlist_lookup_string(rnvl, propname,
>>> +=09=09=09=09=09=09&rvstr) =3D=3D ENOENT)) {
>>> +=09=09=09=09=09goto compare_nums;
>>> +=09=09=09=09}
>>
>> Another thing not to like about ZoL: their completely bogus code style
>> and formatting practices (look at those "&rvstr) =3D=3D ENOENT").  If th=
ey
>> are going to listen to us, can we at least try to convince them not to
>> break existing, much FreeBSD-like formatting?
>
> Actually it is required to use the proper cstyle=E2=80=A6 https://github.=
com/zfsonlinux/zfs/blob/master/.github/CONTRIBUTING.md#style-guides <https:=
//github.com/zfsonlinux/zfs/blob/master/.github/CONTRIBUTING.md#style-guide=
s>
>
> Therefore bogus style is bug.

Indeed, the change was away from the proper cstyle.

Old zfs code is closer to KNF than most FreeBSD code, perhaps because
its cstyle guide is better than style(9) and is actually followed.  It
requires 4-space continuation indents the same as KNF, but these were
broken in the patch.  It requires casts to not be followed by a blank
(i.e., space), except, unfortunately, for function calls whose return
values are ignored.  So the old code above conformed to cstyle but not
to style(9) since it has spaces after "(void)" but otherwise conforms
to KNF.

The cstyle pdf in the above url was written in 1993 and last updated
in 1996.  Its age would be a feature, except it has only primitive
support for C90 (it never mentions "prototype" or ISO C, but has a
couple of examples of "ANSI" function definitions).  So it isn't
actually better than style(9), and I doubt that anyone actually follows
all of it.  Especially its requirement to support K&R in headers and its
examples of incomplete K&R-compatible declarations.

Bruce
From owner-svn-src-all@freebsd.org  Sat May 18 16:19:32 2019
Return-Path: <owner-svn-src-all@freebsd.org>
Delivered-To: svn-src-all@mailman.ysv.freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1])
 by mailman.ysv.freebsd.org (Postfix) with ESMTP id 528FC15919EB;
 Sat, 18 May 2019 16:19:32 +0000 (UTC) (envelope-from kib@FreeBSD.org)
Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org
 [IPv6:2610:1c1:1:606c::19:3])
 (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 "mxrelay.nyi.freebsd.org",
 Issuer "Let's Encrypt Authority X3" (verified OK))
 by mx1.freebsd.org (Postfix) with ESMTPS id E4A3D886E0;
 Sat, 18 May 2019 16:19:31 +0000 (UTC) (envelope-from kib@FreeBSD.org)
Received: from repo.freebsd.org (repo.freebsd.org
 [IPv6:2610:1c1:1:6068::e6a:0])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (Client did not present a certificate)
 by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id BFC6C21F4E;
 Sat, 18 May 2019 16:19:31 +0000 (UTC) (envelope-from kib@FreeBSD.org)
Received: from repo.freebsd.org ([127.0.1.37])
 by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x4IGJVo6095201;
 Sat, 18 May 2019 16:19:31 GMT (envelope-from kib@FreeBSD.org)
Received: (from kib@localhost)
 by repo.freebsd.org (8.15.2/8.15.2/Submit) id x4IGJVF6095199;
 Sat, 18 May 2019 16:19:31 GMT (envelope-from kib@FreeBSD.org)
Message-Id: <201905181619.x4IGJVF6095199@repo.freebsd.org>
X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org
 using -f
From: Konstantin Belousov <kib@FreeBSD.org>
Date: Sat, 18 May 2019 16:19:31 +0000 (UTC)
To: src-committers@freebsd.org, svn-src-all@freebsd.org,
 svn-src-head@freebsd.org
Subject: svn commit: r347957 - head/sys/amd64/amd64
X-SVN-Group: head
X-SVN-Commit-Author: kib
X-SVN-Commit-Paths: head/sys/amd64/amd64
X-SVN-Commit-Revision: 347957
X-SVN-Commit-Repository: base
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-Rspamd-Queue-Id: E4A3D886E0
X-Spamd-Bar: --
Authentication-Results: mx1.freebsd.org
X-Spamd-Result: default: False [-2.96 / 15.00];
 local_wl_from(0.00)[FreeBSD.org];
 NEURAL_HAM_MEDIUM(-1.00)[-0.999,0];
 NEURAL_HAM_LONG(-1.00)[-1.000,0];
 NEURAL_HAM_SHORT(-0.96)[-0.959,0];
 ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]
X-BeenThere: svn-src-all@freebsd.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "SVN commit messages for the entire src tree \(except for &quot;
 user&quot; and &quot; projects&quot; \)" <svn-src-all.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-all>,
 <mailto:svn-src-all-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-all/>;
List-Post: <mailto:svn-src-all@freebsd.org>
List-Help: <mailto:svn-src-all-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-all>,
 <mailto:svn-src-all-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sat, 18 May 2019 16:19:32 -0000

Author: kib
Date: Sat May 18 16:19:31 2019
New Revision: 347957
URL: https://svnweb.freebsd.org/changeset/base/347957

Log:
  Make lock-less delayed invalidation operational very early.
  
  Apparently, there is more code trying to call pmap_remove() early,
  mostly to free preloaded memory.  Instead of moving all deallocations
  to the point where a scheduler is initialized, add missed setup of
  thread0 di init at hammer_time().
  
  The code in pmap_delayed_invl_start_u() is modified to not ever take
  the thread lock if the thread priority is less or equal to PVM.  Since
  thread0 starts at priority 0, and then is reset to PVM at
  proc0_init(), this eliminates taking the thread lock during early
  boot.
  
  While there, fix off by one in comparision of the base priority.
  
  Reported and tested by:	bcran (previous version)
  Reviewed by:	markj
  Sponsored by:	The FreeBSD Foundation
  MFC after:	29 days

Modified:
  head/sys/amd64/amd64/machdep.c
  head/sys/amd64/amd64/pmap.c

Modified: head/sys/amd64/amd64/machdep.c
==============================================================================
--- head/sys/amd64/amd64/machdep.c	Sat May 18 14:55:59 2019	(r347956)
+++ head/sys/amd64/amd64/machdep.c	Sat May 18 16:19:31 2019	(r347957)
@@ -1617,6 +1617,13 @@ hammer_time(u_int64_t modulep, u_int64_t physfree)
 	physfree += kstack0_sz;
 
 	/*
+	 * Initialize enough of thread0 for delayed invalidation to
+	 * work very early.  Rely on thread0.td_base_pri
+	 * zero-initialization, it is reset to PVM at proc0_init().
+	 */
+	pmap_thread_init_invl_gen(&thread0);
+
+	/*
 	 * make gdt memory segments
 	 */
 	for (x = 0; x < NGDT; x++) {

Modified: head/sys/amd64/amd64/pmap.c
==============================================================================
--- head/sys/amd64/amd64/pmap.c	Sat May 18 14:55:59 2019	(r347956)
+++ head/sys/amd64/amd64/pmap.c	Sat May 18 16:19:31 2019	(r347957)
@@ -700,16 +700,17 @@ pmap_delayed_invl_start_u(void)
 	invl_gen = &td->td_md.md_invl_gen;
 	PMAP_ASSERT_NOT_IN_DI();
 	lock_delay_arg_init(&lda, &di_delay);
-	thread_lock(td);
+	invl_gen->saved_pri = 0;
 	pri = td->td_base_pri;
-	if (pri < PVM) {
-		invl_gen->saved_pri = 0;
-	} else {
-		invl_gen->saved_pri = pri;
-		sched_prio(td, PVM);
+	if (pri > PVM) {
+		thread_lock(td);
+		pri = td->td_base_pri;
+		if (pri > PVM) {
+			invl_gen->saved_pri = pri;
+			sched_prio(td, PVM);
+		}
+		thread_unlock(td);
 	}
-	thread_unlock(td);
-
 again:
 	PV_STAT(i = 0);
 	for (p = &pmap_invl_gen_head;; p = prev.next) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190519002125.B1574>