From owner-freebsd-geom@FreeBSD.ORG Sat Jun 26 23:25:03 2004 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 5C2F716A4CE for ; Sat, 26 Jun 2004 23:25:03 +0000 (GMT) Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by mx1.FreeBSD.org (Postfix) with ESMTP id E49D543D3F for ; Sat, 26 Jun 2004 23:25:02 +0000 (GMT) (envelope-from robert@fledge.watson.org) Received: from fledge.watson.org (localhost [127.0.0.1]) by fledge.watson.org (8.12.11/8.12.11) with ESMTP id i5QNO46F064006 for ; Sat, 26 Jun 2004 19:24:04 -0400 (EDT) (envelope-from robert@fledge.watson.org) Received: from localhost (robert@localhost)i5QNO4xp064003 for ; Sat, 26 Jun 2004 19:24:04 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Sat, 26 Jun 2004 19:24:04 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: geom@FreeBSD.org Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Subject: Putting 'mymutex' behind #ifdef WITNESS in geom_io.c X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 26 Jun 2004 23:25:03 -0000 The 'g_up' and 'g_down' workers use a local mutex to cause WITNESS to complain if a geom class method tries to sleep in the I/O path. This is a very good idea, but does have a performance cost. The attached patch pushes the definition and use of mymutex behind #ifdef WITNESS. I ran a simple 'dd' benchmark on a dual-Xeon box running with an SMP kernel, adaptive mutexes, and the 4BSD scheduler. I ran ten sets of 'dd' from /dev/da0 to /dev/null using 512 byte and 1k I/O sizes; I discarded the first value in each as set as an outlier due to pulling in dd, etc. The result was a reliable .22% reduction in the time taken to accomplish 10,000 I/O operations. While this isn't a huge amount, if you get about ten small optimizations, you've won a couple of percent of performance. Assuming there are no objections, I'm happy to commit it. Here are the numbers; the samples are seconds to complete the 10,000 I/O's per above. x before/512.txt + after/512.txt +--------------------------------------------------------------------------+ | + | |+ + + +++ +* x x x x x xx x| | |___________A____M______| |_____________A_M___________| | +--------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 9 0.820367 0.825714 0.823746 0.82346833 0.0018671501 + 9 0.815945 0.820384 0.819654 0.81901578 0.0016033332 Difference at 95.0% confidence -0.00445256 +/- 0.00173916 -0.540708% +/- 0.2112% (Student's t, pooled s = 0.00174025) x before/1k.txt + after/1k.txt +--------------------------------------------------------------------------+ | + | |+ ++ ++x ++ +x x xxxx x x| | |_________A___M______| |_____________AM____________| | +--------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 9 0.857779 0.865484 0.861588 0.86142444 0.0021438014 + 9 0.854564 0.859112 0.857424 0.85687789 0.0016247486 Difference at 95.0% confidence -0.00454656 +/- 0.00190088 -0.527795% +/- 0.220667% (Student's t, pooled s = 0.00190206) Robert N M Watson FreeBSD Core Team, TrustedBSD Projects robert@fledge.watson.org Principal Research Scientist, McAfee Research ==== //depot/user/rwatson/netperf/sys/geom/geom_io.c#5 - /data/p4/rwatson/rwatson_netperf/sys/geom/geom_io.c ==== @@ -315,10 +315,12 @@ struct bio *bp; off_t excess; int error; +#ifdef WITNESS struct mtx mymutex; bzero(&mymutex, sizeof mymutex); mtx_init(&mymutex, "g_xdown", NULL, MTX_DEF); +#endif for(;;) { g_bioq_lock(&g_bio_run_down); @@ -357,9 +359,13 @@ default: break; } +#ifdef WITNESS mtx_lock(&mymutex); +#endif bp->bio_to->geom->start(bp); +#ifdef WITNESS mtx_unlock(&mymutex); +#endif } } @@ -384,26 +390,36 @@ g_io_schedule_up(struct thread *tp __unused) { struct bio *bp; +#ifdef WITNESS struct mtx mymutex; bzero(&mymutex, sizeof mymutex); mtx_init(&mymutex, "g_xup", NULL, MTX_DEF); +#endif for(;;) { g_bioq_lock(&g_bio_run_up); bp = g_bioq_first(&g_bio_run_task); if (bp != NULL) { g_bioq_unlock(&g_bio_run_up); +#ifdef WITNESS mtx_lock(&mymutex); +#endif bp->bio_task(bp->bio_task_arg); +#ifdef WITNESS mtx_unlock(&mymutex); +#endif continue; } bp = g_bioq_first(&g_bio_run_up); if (bp != NULL) { g_bioq_unlock(&g_bio_run_up); +#ifdef WITNESS mtx_lock(&mymutex); +#endif biodone(bp); +#ifdef WITNESS mtx_unlock(&mymutex); +#endif continue; } msleep(&g_wait_up, &g_bio_run_up.bio_queue_lock,