Date: Wed, 12 Mar 2008 07:45:33 -0400 From: Coleman Kane <cokane@cokane.org> To: arch@FreeBSD.org Subject: SMPTODO: remove timeout(9) from ffs_softdep.c Message-ID: <47D7C25D.5070908@cokane.org>
next in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------000300090804080100080401
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Hi all,
I was poking around SMPTODO for some work during an idle night, and I
decided to fix the non-MPSAFE use of timeout(9) in ffs_softdep.c, and
learn more about the callout_* API in the kernel. I'm attaching a patch
of what I've done, which I am running in my current kernel at the moment
(and I am using softupdates on a number of filesystems on this SMP machine).
Can anyone else try it out / review it / give feedback?
--
Coleman Kane
--------------000300090804080100080401
Content-Type: text/x-patch;
name="ffs_softdep.c-newcallout.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="ffs_softdep.c-newcallout.diff"
diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
index 3e8ba26..3e9122f 100644
--- a/sys/ufs/ffs/ffs_softdep.c
+++ b/sys/ufs/ffs/ffs_softdep.c
@@ -664,7 +664,7 @@ static int maxindirdeps = 50; /* max number of indirdeps before slowdown */
static int tickdelay = 2; /* number of ticks to pause during slowdown */
static int proc_waiting; /* tracks whether we have a timeout posted */
static int *stat_countp; /* statistic to count in proc_waiting timeout */
-static struct callout_handle handle; /* handle on posted proc_waiting timeout */
+static struct callout softdep_callout;
static int req_pending;
static int req_clear_inodedeps; /* syncer process flush some inodedeps */
#define FLUSH_INODES 1
@@ -1394,6 +1394,9 @@ softdep_initialize()
bioops.io_complete = softdep_disk_write_complete;
bioops.io_deallocate = softdep_deallocate_dependencies;
bioops.io_countdeps = softdep_count_dependencies;
+
+ /* Initialize the callout with an mtx. */
+ callout_init_mtx(&softdep_callout, &lk, 0);
}
/*
@@ -1403,7 +1406,9 @@ softdep_initialize()
void
softdep_uninitialize()
{
-
+ ACQUIRE_LOCK(&lk);
+ callout_drain(&softdep_callout);
+ FREE_LOCK(&lk);
hashdestroy(pagedep_hashtbl, M_PAGEDEP, pagedep_hash);
hashdestroy(inodedep_hashtbl, M_INODEDEP, inodedep_hash);
hashdestroy(newblk_hashtbl, M_NEWBLK, newblk_hash);
@@ -5858,8 +5863,16 @@ request_cleanup(mp, resource)
* We wait at most tickdelay before proceeding in any case.
*/
proc_waiting += 1;
- if (handle.callout == NULL)
- handle = timeout(pause_timer, 0, tickdelay > 2 ? tickdelay : 2);
+ ACQUIRE_LOCK(&lk);
+ if(callout_active(&softdep_callout) == FALSE) {
+ /*
+ should always return zero due to callout_active being called to verify that no active
+ timeout already exists, which is the case where this would return non-zero (and
+ callout_active(&softdep_callout) would be TRUE.
+ */
+ callout_reset(&softdep_callout, tickdelay > 2 ? tickdelay : 2, pause_timer, 0);
+ }
+ FREE_LOCK(&lk);
msleep((caddr_t)&proc_waiting, &lk, PPAUSE, "softupdate", 0);
proc_waiting -= 1;
return (1);
@@ -5873,15 +5886,17 @@ static void
pause_timer(arg)
void *arg;
{
-
- ACQUIRE_LOCK(&lk);
+ /* Implied by callout_* API */
+ /* ACQUIRE_LOCK(&lk); */
*stat_countp += 1;
wakeup_one(&proc_waiting);
- if (proc_waiting > 0)
- handle = timeout(pause_timer, 0, tickdelay > 2 ? tickdelay : 2);
- else
- handle.callout = NULL;
- FREE_LOCK(&lk);
+ if (proc_waiting > 0) {
+ /* We don't care about the return value here. */
+ callout_reset(&softdep_callout, tickdelay > 2 ? tickdelay : 2, pause_timer, 0);
+ } else {
+ callout_deactivate(&softdep_callout);
+ }
+ /* FREE_LOCK(&lk); */
}
/*
--------------000300090804080100080401--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?47D7C25D.5070908>