From owner-svn-src-all@freebsd.org  Sat Jun  9 17:15:30 2018
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 4D1AC1011151;
 Sat,  9 Jun 2018 17:15:30 +0000 (UTC)
 (envelope-from danfe@freebsd.org)
Received: from freefall.freebsd.org (freefall.freebsd.org
 [IPv6:2610:1c1:1:6074::16:84])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (Client CN "freefall.freebsd.org",
 Issuer "Let's Encrypt Authority X3" (verified OK))
 by mx1.freebsd.org (Postfix) with ESMTPS id CC531866E3;
 Sat,  9 Jun 2018 12:53:00 +0000 (UTC)
 (envelope-from danfe@freebsd.org)
Received: by freefall.freebsd.org (Postfix, from userid 1033)
 id C522C87A4; Sat,  9 Jun 2018 12:53:00 +0000 (UTC)
Date: Sat, 9 Jun 2018 12:53:00 +0000
From: Alexey Dokuchaev <danfe@FreeBSD.org>
To: Sean Eric Fagan <sef@FreeBSD.org>
Cc: src-committers@freebsd.org, svn-src-all@freebsd.org,
 svn-src-head@freebsd.org
Subject: Re: svn commit: r334844 - in head: cddl/contrib/opensolaris/cmd/zdb
 cddl/contrib/opensolaris/cmd/zpool cddl/contrib/opensolaris/cmd/ztest
 cddl/contrib/opensolaris/lib/libzfs/common cddl/contrib/opensol...
Message-ID: <20180609125300.GA97439@FreeBSD.org>
References: <201806081738.w58HcSsM094656@repo.freebsd.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <201806081738.w58HcSsM094656@repo.freebsd.org>
User-Agent: Mutt/1.9.5 (2018-04-13)
X-BeenThere: svn-src-all@freebsd.org
X-Mailman-Version: 2.1.26
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, 09 Jun 2018 17:15:30 -0000

On Fri, Jun 08, 2018 at 05:38:28PM +0000, Sean Eric Fagan wrote:
> New Revision: 334844
> URL: https://svnweb.freebsd.org/changeset/base/334844
> 
> Log:
>   This originated from ZFS On Linux [...]
>   
>   During scans (scrubs or resilvers), it sorts the blocks in each
>   transaction group by block offset; the result can be a significant
>   improvement.

That's pretty cool, thanks for bringing this in!  Couple of comments
about the commit itself (not the code).

> @@ -2281,14 +2281,14 @@ dump_dir(objset_t *os)
>  		object_count++;
>  	}
>  
> -	ASSERT3U(object_count, ==, usedobjs);
> -
>  	(void) printf("\n");
>  
>  	if (error != ESRCH) {
>  		(void) fprintf(stderr, "dmu_object_next() = %d\n", error);
>  		abort();
>  	}
> +
> +	ASSERT3U(object_count, ==, usedobjs);

This seems to be about something else, not the sorting of blocks, is it?

> -	if (ps && ps->pss_state == DSS_SCANNING &&
> +	if (ps != NULL && ps->pss_state == DSS_SCANNING &&
...
> -	/*
> -	 * Scan is finished or canceled.
> -	 */
> -	if (ps->pss_state == DSS_FINISHED) {
> -		uint64_t minutes_taken = (end - start) / 60;
> -		char *fmt = NULL;
>  
> +	/* Scan is finished or canceled. */
> +	if (ps->pss_state == DSS_FINISHED) {

Ideally, style fixes should be committed separately from the functional
changes.

> -	if (ps && ps->pss_func == POOL_SCAN_RESILVER &&
> +	if (ps != NULL && ps->pss_func == POOL_SCAN_RESILVER &&
>  	    ps->pss_state == DSS_SCANNING)
>  		return (ZPOOL_STATUS_RESILVERING);

There're not awfully many of them, but still.

./danfe