From owner-svn-src-head@freebsd.org Fri Aug 14 17:09:02 2015 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id ABB629B80ED; Fri, 14 Aug 2015 17:09:02 +0000 (UTC) (envelope-from mahrens@gmail.com) Received: from mail-la0-x22c.google.com (mail-la0-x22c.google.com [IPv6:2a00:1450:4010:c03::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 3147813B6; Fri, 14 Aug 2015 17:09:02 +0000 (UTC) (envelope-from mahrens@gmail.com) Received: by lahi9 with SMTP id i9so47233804lah.2; Fri, 14 Aug 2015 10:09:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=YFyueoqwq024V+DrgvZwdml6l3Rx4iZX4kUR3LNwcPo=; b=mL7DFzX4gMjJFSRatQB6Dyx2xELSJoRHIZNZRDEfOhouUPyWSNSguADGdgOzEE13y5 kHF5mS+r8uVE/0PsiXUiI64kay2vPdkjsArSGgJU7ToEPjJhChyD7RVqaxWF6mXYCoGf SEvKS+HpVcFRYxCl399V+877ZiY+eLdryNAWdezcDhTKsY/8CtGAYvMmAv7BwdTllPSJ AVc5hrjyJLVRO6g/ci0z7UNV7bOzDjFpHAe50GA2kIe6wFz3JJosjX5QOSAPTAEOcNCT bKS6kDUwSZG0hzDyg7W7orEWJdYuagXkU8CV6BV5kA1S93awPQ/mZcb7jEF+Fic/aqIR 1sgQ== MIME-Version: 1.0 X-Received: by 10.112.63.169 with SMTP id h9mr44985538lbs.104.1439572140013; Fri, 14 Aug 2015 10:09:00 -0700 (PDT) Sender: mahrens@gmail.com Received: by 10.112.163.65 with HTTP; Fri, 14 Aug 2015 10:08:59 -0700 (PDT) In-Reply-To: <55CDF9ED.4090908@FreeBSD.org> References: <201501201309.t0KD9Cid002762@svn.freebsd.org> <55CDF9ED.4090908@FreeBSD.org> Date: Fri, 14 Aug 2015 10:08:59 -0700 X-Google-Sender-Auth: SmqqvaNeLPzlmgE4yZVKhfG01tk Message-ID: Subject: Re: svn commit: r277419 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs From: Matthew Ahrens To: Andriy Gapon Cc: Alexander Motin , "src-committers@freebsd.org" , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Aug 2015 17:09:02 -0000 On Fri, Aug 14, 2015 at 7:23 AM, Andriy Gapon wrote: > On 20/01/2015 15:09, Alexander Motin wrote: > > Author: mav > > Date: Tue Jan 20 13:09:12 2015 > > New Revision: 277419 > > URL: https://svnweb.freebsd.org/changeset/base/277419 > > > > Log: > > Allow skipping dmu_buf_will_dirty() call in dsl_dir_transfer_space(). > > > > dsl_dir_transfer_space() is mostly called after dsl_dir_diduse_space(), > > which already calls dmu_buf_will_dirty() for the same dbuf and tx, so > > its duplicate call in those cases will change nothing, only spend time. > > > > Skipping this call by four times reduces time spent in > dbuf_write_done() > > and descendants, updating dataset statistics with several congested > lock > > acquisitions. When rewriting 8K zvol blocks at 1GB/s rate, this > reduces > > CPU time spent inside dbuf_write_done(), according to profiling, from > 45% > > of 683K samples to 18% of 422K. > > It probably would make sense to discuss such a change with the upstream > folks. Or, at the very least, let them know about it. > I have observed similar perf issues and have fixed (in a private, not-fully-tested branch) in a similar way. These changes depend on the fact that the dbuf is already dirty. To ensure that this remains true in the future, we should add assertions (which run on debug bits only) that it is already dirty, and add comments explaining where it was dirtied, and add a comment where it is dirtied explaining the other places that depend on the dirtying. --matt > > > > MFC after: 2 weeks > > > > Modified: > > head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c > > head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c > > > > Modified: > head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c > > > ============================================================================== > > --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c > Tue Jan 20 12:28:24 2015 (r277418) > > +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c > Tue Jan 20 13:09:12 2015 (r277419) > > @@ -136,7 +136,7 @@ dsl_dataset_block_born(dsl_dataset_t *ds > > dsl_dir_diduse_space(ds->ds_dir, DD_USED_HEAD, delta, > > compressed, uncompressed, tx); > > dsl_dir_transfer_space(ds->ds_dir, used - delta, > > - DD_USED_REFRSRV, DD_USED_HEAD, tx); > > + DD_USED_REFRSRV, DD_USED_HEAD, NULL); > > } > > > > int > > @@ -179,7 +179,7 @@ dsl_dataset_block_kill(dsl_dataset_t *ds > > dsl_dir_diduse_space(ds->ds_dir, DD_USED_HEAD, > > delta, -compressed, -uncompressed, tx); > > dsl_dir_transfer_space(ds->ds_dir, -used - delta, > > - DD_USED_REFRSRV, DD_USED_HEAD, tx); > > + DD_USED_REFRSRV, DD_USED_HEAD, NULL); > > } else { > > dprintf_bp(bp, "putting on dead list: %s", ""); > > if (async) { > > @@ -2837,7 +2837,7 @@ dsl_dataset_clone_swap_sync_impl(dsl_dat > > origin_head->ds_dir->dd_origin_txg, UINT64_MAX, > > &odl_used, &odl_comp, &odl_uncomp); > > dsl_dir_transfer_space(origin_head->ds_dir, cdl_used - > odl_used, > > - DD_USED_HEAD, DD_USED_SNAP, tx); > > + DD_USED_HEAD, DD_USED_SNAP, NULL); > > } > > > > /* swap ds_*_bytes */ > > > > Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c > > > ============================================================================== > > --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c > Tue Jan 20 12:28:24 2015 (r277418) > > +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c > Tue Jan 20 13:09:12 2015 (r277419) > > @@ -1389,7 +1389,7 @@ dsl_dir_diduse_space(dsl_dir_t *dd, dd_u > > accounted_delta, compressed, uncompressed, tx); > > dsl_dir_transfer_space(dd->dd_parent, > > used - accounted_delta, > > - DD_USED_CHILD_RSRV, DD_USED_CHILD, tx); > > + DD_USED_CHILD_RSRV, DD_USED_CHILD, NULL); > > } > > } > > > > @@ -1397,7 +1397,7 @@ void > > dsl_dir_transfer_space(dsl_dir_t *dd, int64_t delta, > > dd_used_t oldtype, dd_used_t newtype, dmu_tx_t *tx) > > { > > - ASSERT(dmu_tx_is_syncing(tx)); > > + ASSERT(tx == NULL || dmu_tx_is_syncing(tx)); > > ASSERT(oldtype < DD_USED_NUM); > > ASSERT(newtype < DD_USED_NUM); > > > > @@ -1405,7 +1405,8 @@ dsl_dir_transfer_space(dsl_dir_t *dd, in > > !(dsl_dir_phys(dd)->dd_flags & DD_FLAG_USED_BREAKDOWN)) > > return; > > > > - dmu_buf_will_dirty(dd->dd_dbuf, tx); > > + if (tx != NULL) > > + dmu_buf_will_dirty(dd->dd_dbuf, tx); > > mutex_enter(&dd->dd_lock); > > ASSERT(delta > 0 ? > > dsl_dir_phys(dd)->dd_used_breakdown[oldtype] >= delta : > > > > > -- > Andriy Gapon > >