From owner-freebsd-hackers@freebsd.org Thu Nov 30 14:58:19 2017 Return-Path: Delivered-To: freebsd-hackers@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 8E445E5D973 for ; Thu, 30 Nov 2017 14:58:19 +0000 (UTC) (envelope-from andre@fbsd.e4m.org) Received: from mail.g66.org (mail.g66.org [85.10.206.112]) by mx1.freebsd.org (Postfix) with ESMTP id 3889C67240; Thu, 30 Nov 2017 14:58:18 +0000 (UTC) (envelope-from andre@fbsd.e4m.org) Received: from x55b53023.dyn.telefonica.de (x55b53023.dyn.telefonica.de [85.181.48.35]) (authenticated bits=128) by mail.g66.org (8.15.2/8.15.2) with ESMTPA id vAUEwAMa034912; Thu, 30 Nov 2017 15:58:11 +0100 (CET) (envelope-from andre@fbsd.e4m.org) Received: from submit.client ([127.0.0.1]) by gate.local (8.15.2/8.15.2) with ESMTP id vAUEwAlF090779; Thu, 30 Nov 2017 15:58:10 +0100 (CET) (envelope-from andre@fbsd.e4m.org) Received: (from user@localhost) by gate.local (8.15.2/8.15.2/Submit) id vAUEwA17090778; Thu, 30 Nov 2017 15:58:10 +0100 (CET) (envelope-from andre@fbsd.e4m.org) Date: Thu, 30 Nov 2017 15:58:10 +0100 From: Andre Albsmeier To: Mark Johnston Cc: Andre Albsmeier , freebsd-hackers@freebsd.org Subject: Re: gmirror synchronising is very slow due to frequent metadata updates Message-ID: <20171130145810.GA90581@gate> References: <20171119103241.GA20588@voyager> <20171120033828.GA1959@bish> <20171120053409.GA57536@gate> <20171121173813.GB4126@raichu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171121173813.GB4126@raichu> User-Agent: Mutt/1.7.2 (2016-11-26) X-Virus-Scanned: clamav-milter 0.99.2 at colo X-Virus-Status: Clean X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Nov 2017 14:58:19 -0000 On Tue, 21-Nov-2017 at 12:38:13 -0500, Mark Johnston wrote: > On Mon, Nov 20, 2017 at 06:34:09AM +0100, Andre Albsmeier wrote: > > On Sun, 19-Nov-2017 at 22:38:28 -0500, Mark Johnston wrote: > > > We should probably decrease the update interval based on the size of a > > > mirror. For mirrors larger than say, 1GB, we might just update the > > > metadata block once per 1% of the synchronization operation's progress. > > > > I think best would be to have it updated every seconds. > > I think of very fast or very slow drives, or drives which are used > > heavily during rebuild (where the effective rebuild speed is quite > > low)... > > I think that's reasonable. Could you give the patch below a try? It adds I am some more weeks in the jungle with nothing more than two notebooks so trying your patch will be a bit hard (and, call me a coward, I won't do this on my mirror'ed machines 8000 km away ;-)). Apart from that it looks quite simple and seems to do all what we need (my impression as a non-kernel hacker). Maybe someone else can review it and we just give it a try? Thanks, and sorry for the late reply... -Andre > a sysctl which configures the frequency of metadata updates during > synchronization. It'd be nice to be able to specify this parameter (as > well as idletime) on a per-gmirror basis, with the default value still > being taken from the sysctl, but that can be implemented separately. > > diff --git a/sys/geom/mirror/g_mirror.c b/sys/geom/mirror/g_mirror.c > index 4a5f542c1de5..47d11f3ce465 100644 > --- a/sys/geom/mirror/g_mirror.c > +++ b/sys/geom/mirror/g_mirror.c > @@ -69,6 +69,10 @@ SYSCTL_UINT(_kern_geom_mirror, OID_AUTO, disconnect_on_failure, CTLFLAG_RWTUN, > static u_int g_mirror_syncreqs = 2; > SYSCTL_UINT(_kern_geom_mirror, OID_AUTO, sync_requests, CTLFLAG_RDTUN, > &g_mirror_syncreqs, 0, "Parallel synchronization I/O requests."); > +static u_int g_mirror_sync_period = 5; > +SYSCTL_UINT(_kern_geom_mirror, OID_AUTO, sync_update_period, CTLFLAG_RWTUN, > + &g_mirror_sync_period, 0, > + "Metadata update period during synchroniztion, in seconds"); > > #define MSLEEP(ident, mtx, priority, wmesg, timeout) do { \ > G_MIRROR_DEBUG(4, "%s: Sleeping %p.", __func__, (ident)); \ > @@ -463,6 +467,7 @@ g_mirror_init_disk(struct g_mirror_softc *sc, struct g_provider *pp, > disk->d_sync.ds_consumer = NULL; > disk->d_sync.ds_offset = md->md_sync_offset; > disk->d_sync.ds_offset_done = md->md_sync_offset; > + disk->d_sync.ds_update_ts = time_uptime; > disk->d_genid = md->md_genid; > disk->d_sync.ds_syncid = md->md_syncid; > if (errorp != NULL) > @@ -1457,10 +1462,11 @@ g_mirror_sync_request(struct bio *bp) > if (bp != NULL && bp->bio_offset < offset) > offset = bp->bio_offset; > } > - if (sync->ds_offset_done + (MAXPHYS * 100) < offset) { > - /* Update offset_done on every 100 blocks. */ > + if (g_mirror_sync_period > 0 && > + time_uptime - sync->ds_update_ts > g_mirror_sync_period) { > sync->ds_offset_done = offset; > g_mirror_update_metadata(disk); > + sync->ds_update_ts = time_uptime; > } > return; > } > diff --git a/sys/geom/mirror/g_mirror.h b/sys/geom/mirror/g_mirror.h > index 3b9664035c98..be46f7b43e4c 100644 > --- a/sys/geom/mirror/g_mirror.h > +++ b/sys/geom/mirror/g_mirror.h > @@ -108,6 +108,7 @@ struct g_mirror_disk_sync { > off_t ds_offset; /* Offset of next request to send. */ > off_t ds_offset_done; /* Offset of already synchronized > region. */ > + time_t ds_update_ts; /* Time of last metadata update. */ > u_int ds_syncid; /* Disk's synchronization ID. */ > u_int ds_inflight; /* Number of in-flight sync requests. */ > struct bio **ds_bios; /* BIOs for synchronization I/O. */ -- FreeBSD-11.0: Another day closer to a Micro$oft free world