From owner-svn-src-head@FreeBSD.ORG Tue Sep 24 17:27:30 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id C4C2EF9D; Tue, 24 Sep 2013 17:27:30 +0000 (UTC) (envelope-from lists@yamagi.org) Received: from mail1.yamagi.org (yugo.yamagi.org [84.201.39.245]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 578A62C4A; Tue, 24 Sep 2013 17:27:29 +0000 (UTC) Received: from pd95234d5.dip0.t-ipconnect.de ([217.82.52.213] helo=maka.home.yamagi.org) by mail1.yamagi.org with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.80.1 (FreeBSD)) (envelope-from ) id 1VNHLN-000NCV-1k; Sat, 21 Sep 2013 09:11:07 +0200 Date: Sat, 21 Sep 2013 09:10:58 +0200 From: Yamagi Burmeister To: trociny@FreeBSD.org Subject: Re: svn commit: r255716 - head/sbin/hastd Message-Id: <20130921091058.5137e5dcbea8b628c81f5207@yamagi.org> In-Reply-To: <201309192019.r8JKJ9Sw001364@svn.freebsd.org> References: <201309192019.r8JKJ9Sw001364@svn.freebsd.org> X-Mailer: Sylpheed 3.3.0 (GTK+ 2.24.19; amd64-portbld-freebsd8.4) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 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: Tue, 24 Sep 2013 17:27:30 -0000 Thank you for your hard work and help :) On Thu, 19 Sep 2013 20:19:09 +0000 (UTC) Mikolaj Golub wrote: > Author: trociny > Date: Thu Sep 19 20:19:08 2013 > New Revision: 255716 > URL: http://svnweb.freebsd.org/changeset/base/255716 > > Log: > When updating the map of dirty extents, most recently used extents are > kept dirty to reduce the number of on-disk metadata updates. The > sequence of operations is: > > 1) acquire the activemap lock; > 2) update in-memory map; > 3) if the list of keepdirty extents is changed, update on-disk metadata; > 4) release the lock. > > On-disk updates are not frequent in comparison with in-memory updates, > while require much more time. So situations are possible when one > thread is updating on-disk metadata and another one is waiting for the > activemap lock just to update the in-memory map. > > Improve this by introducing additional, on-disk map lock: when > in-memory map is updated and it is detected that the on-disk map needs > update too, the on-disk map lock is acquired and the on-memory lock is > released before flushing the map. > > Reported by: Yamagi Burmeister yamagi.org > Tested by: Yamagi Burmeister yamagi.org > Reviewed by: pjd > Approved by: re (marius) > MFC after: 2 weeks > > Modified: > head/sbin/hastd/hast.h > head/sbin/hastd/primary.c > > Modified: head/sbin/hastd/hast.h > ============================================================================== > --- head/sbin/hastd/hast.h Thu Sep 19 20:17:50 2013 (r255715) > +++ head/sbin/hastd/hast.h Thu Sep 19 20:19:08 2013 (r255716) > @@ -226,8 +226,10 @@ struct hast_resource { > > /* Activemap structure. */ > struct activemap *hr_amp; > - /* Locked used to synchronize access to hr_amp. */ > + /* Lock used to synchronize access to hr_amp. */ > pthread_mutex_t hr_amp_lock; > + /* Lock used to synchronize access to hr_amp diskmap. */ > + pthread_mutex_t hr_amp_diskmap_lock; > > /* Number of BIO_READ requests. */ > uint64_t hr_stat_read; > > Modified: head/sbin/hastd/primary.c > ============================================================================== > --- head/sbin/hastd/primary.c Thu Sep 19 20:17:50 2013 (r255715) > +++ head/sbin/hastd/primary.c Thu Sep 19 20:19:08 2013 (r255716) > @@ -291,22 +291,28 @@ primary_exitx(int exitcode, const char * > exit(exitcode); > } > > +/* Expects res->hr_amp locked, returns unlocked. */ > static int > hast_activemap_flush(struct hast_resource *res) > { > const unsigned char *buf; > size_t size; > + int ret; > > + mtx_lock(&res->hr_amp_diskmap_lock); > buf = activemap_bitmap(res->hr_amp, &size); > + mtx_unlock(&res->hr_amp_lock); > PJDLOG_ASSERT(buf != NULL); > PJDLOG_ASSERT((size % res->hr_local_sectorsize) == 0); > + ret = 0; > if (pwrite(res->hr_localfd, buf, size, METADATA_SIZE) != > (ssize_t)size) { > pjdlog_errno(LOG_ERR, "Unable to flush activemap to disk"); > res->hr_stat_activemap_write_error++; > - return (-1); > + ret = -1; > } > - if (res->hr_metaflush == 1 && g_flush(res->hr_localfd) == -1) { > + if (ret == 0 && res->hr_metaflush == 1 && > + g_flush(res->hr_localfd) == -1) { > if (errno == EOPNOTSUPP) { > pjdlog_warning("The %s provider doesn't support flushing write cache. Disabling it.", > res->hr_localpath); > @@ -315,10 +321,11 @@ hast_activemap_flush(struct hast_resourc > pjdlog_errno(LOG_ERR, > "Unable to flush disk cache on activemap update"); > res->hr_stat_activemap_flush_error++; > - return (-1); > + ret = -1; > } > } > - return (0); > + mtx_unlock(&res->hr_amp_diskmap_lock); > + return (ret); > } > > static bool > @@ -783,6 +790,7 @@ init_remote(struct hast_resource *res, s > * Now that we merged bitmaps from both nodes, flush it to the > * disk before we start to synchronize. > */ > + mtx_lock(&res->hr_amp_lock); > (void)hast_activemap_flush(res); > } > nv_free(nvin); > @@ -1288,8 +1296,9 @@ ggate_recv_thread(void *arg) > ggio->gctl_offset, ggio->gctl_length)) { > res->hr_stat_activemap_update++; > (void)hast_activemap_flush(res); > + } else { > + mtx_unlock(&res->hr_amp_lock); > } > - mtx_unlock(&res->hr_amp_lock); > break; > case BIO_DELETE: > res->hr_stat_delete++; > @@ -1650,8 +1659,9 @@ done_queue: > if (activemap_need_sync(res->hr_amp, ggio->gctl_offset, > ggio->gctl_length)) { > (void)hast_activemap_flush(res); > + } else { > + mtx_unlock(&res->hr_amp_lock); > } > - mtx_unlock(&res->hr_amp_lock); > if (hio->hio_replication == HAST_REPLICATION_MEMSYNC) > (void)refcnt_release(&hio->hio_countdown); > } > @@ -1918,8 +1928,9 @@ ggate_send_thread(void *arg) > ggio->gctl_offset, ggio->gctl_length)) { > res->hr_stat_activemap_update++; > (void)hast_activemap_flush(res); > + } else { > + mtx_unlock(&res->hr_amp_lock); > } > - mtx_unlock(&res->hr_amp_lock); > } > if (ggio->gctl_cmd == BIO_WRITE) { > /* > @@ -2015,8 +2026,11 @@ sync_thread(void *arg __unused) > */ > if (activemap_extent_complete(res->hr_amp, syncext)) > (void)hast_activemap_flush(res); > + else > + mtx_unlock(&res->hr_amp_lock); > + } else { > + mtx_unlock(&res->hr_amp_lock); > } > - mtx_unlock(&res->hr_amp_lock); > if (dorewind) { > dorewind = false; > if (offset == -1) > _______________________________________________ > svn-src-all@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/svn-src-all > To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" -- Homepage: www.yamagi.org XMPP: yamagi@yamagi.org GnuPG/GPG: 0xEFBCCBCB