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...

Alexey Dokuchaev danfe at FreeBSD.org
Sat Jun 9 17:15:30 UTC 2018


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


More information about the svn-src-all mailing list