Print this page
Update from fsd_sep3 webrev to fsd_sep9
@@ -135,11 +135,11 @@
*
* 4. Internals
* When fsd_enabled is nonzero, fsd_detach() fails.
*
* These mount callback is used for installing injections on newly mounted
- * vfs_t's (omnipresent).
+ * vfs_t's (omnipresent). The free callback is used for cleaning up.
*
* The list of currently installed hooks is kept in fsd_list.
*
* fsd installs at most one hook on a vfs_t.
*
@@ -159,11 +159,11 @@
* (fsd_remove_cb).
*
* Because of the fact that fsd_remove_cb() could be called both in the context
* of the thread that executes fsh_hook_remove() or outside the fsd, we need to
* use fsd_rem_thread in order not to cause a deadlock. fsh_hook_remove() could
- * be called by at most one thread inside fsd (fsd_remove_disturber() holds
+ * be called by at most one thread inside fsd (fsd_disturber_remove() holds
* fsd_lock). We just have to check inside fsd_remove_cb() if it was called
* from fsh_hook_remove() or not. We use fsd_rem_thread to determine that.
*
* fsd_int_t.fsdi_param is protected by fsd_int_t.fsdi_lock which is an rwlock.
*/
@@ -178,11 +178,11 @@
krwlock_t fsdi_lock; /* protects fsd_param */
fsd_t fsdi_param;
fsh_handle_t fsdi_handle; /* we use fsh's handle in fsd */
vfs_t *fsdi_vfsp;
int fsdi_doomed;
- list_node_t fsdi_next;
+ list_node_t fsdi_node;
} fsd_int_t;
static dev_info_t *fsd_devi;
@@ -230,63 +230,86 @@
/*
* A pointer to a given fsd_int_t is valid always inside fsh_hook_xxx()
* call, because it's valid until the hooks associated with it are removed.
* If a hook is removed, it cannot be executing.
*/
-static int
-fsd_hook_read(fsh_int_t *fshi, void *arg, vnode_t *vp, uio_t *uiop,
- int ioflag, cred_t *cr, caller_context_t *ct)
+static void
+fsd_hook_pre_read(void *arg, void **instancep, vnode_t **vpp, uio_t **uiopp,
+ int *ioflagp, cred_t **crp, caller_context_t **ctp)
{
+ _NOTE(ARGUNUSED(ioflagp));
+ _NOTE(ARGUNUSED(crp));
+ _NOTE(ARGUNUSED(ctp));
+
fsd_int_t *fsdi = (fsd_int_t *)arg;
- uint64_t count, less, less_chance;
+ uint64_t less_chance;
/*
* It is used to keep an odd number of fsd_rand() calls in every
- * fsd_hook_read() call. That is desired because when a range of width
- * 2 is set as a parameter, we don't want to make it a constant.
+ * fsd_hook_pre_read() call. That is desired because when a range of
+ * width 2 is set as a parameter, we don't want to make it a constant.
* The pseudo-random number generator returns a number with different
* parity with every call. If this function is called in every
- * fsd_hook_read() execution even number of times, it would always be
- * the same % 2.
+ * fsd_hook_pre_read() execution even number of times, it would always
+ * be the same % 2.
*/
(void) fsd_rand();
- ASSERT(vp->v_vfsp == fsdi->fsdi_vfsp);
+ ASSERT((*vpp)->v_vfsp == fsdi->fsdi_vfsp);
rw_enter(&fsdi->fsdi_lock, RW_READER);
less_chance = fsdi->fsdi_param.read_less_chance;
- less = (uint64_t)fsd_rand() %
- (fsdi->fsdi_param.read_less_r[1] + 1 -
- fsdi->fsdi_param.read_less_r[0]) + fsdi->fsdi_param.read_less_r[0];
rw_exit(&fsdi->fsdi_lock);
- count = uiop->uio_iov->iov_len;
if ((uint64_t)fsd_rand() % 100 < less_chance) {
extern size_t copyout_max_cached;
- int ret;
+ uint64_t r[2];
+ uint64_t count, less;
- if (count > less)
+ count = (*uiopp)->uio_iov->iov_len;
+ r[0] = fsdi->fsdi_param.read_less_r[0];
+ r[1] = fsdi->fsdi_param.read_less_r[1];
+ less = (uint64_t)fsd_rand() % (r[1] + 1 - r[0]) + r[0];
+
+ if (count > less) {
count -= less;
- else
- less = 0;
+ *instancep = kmem_alloc(sizeof (uint64_t), KM_SLEEP);
+ *(*(uint64_t **)instancep) = less;
+ } else {
+ *instancep = NULL;
+ return;
+ }
- uiop->uio_iov->iov_len = count;
- uiop->uio_resid = count;
+ (*uiopp)->uio_iov->iov_len = count;
+ (*uiopp)->uio_resid = count;
if (count <= copyout_max_cached)
- uiop->uio_extflg = UIO_COPY_CACHED;
+ (*uiopp)->uio_extflg = UIO_COPY_CACHED;
else
- uiop->uio_extflg = UIO_COPY_DEFAULT;
-
- ret = fsh_next_read(fshi, vp, uiop, ioflag, cr, ct);
- uiop->uio_resid += less;
- return (ret);
+ (*uiopp)->uio_extflg = UIO_COPY_DEFAULT;
+ } else {
+ *instancep = NULL;
}
-
- return (fsh_next_read(fshi, vp, uiop, ioflag, cr, ct));
}
+static int
+fsd_hook_post_read(int ret, void *arg, void *instance, vnode_t *vp,
+ uio_t *uiop, int oflag, cred_t *cr, caller_context_t *ct)
+{
+ _NOTE(ARGUNUSED(arg));
+ _NOTE(ARGUNUSED(vp));
+ _NOTE(ARGUNUSED(oflag));
+ _NOTE(ARGUNUSED(cr));
+ _NOTE(ARGUNUSED(ct));
+ if (instance != NULL) {
+ uint64_t *lessp = instance;
+ uiop->uio_resid += *lessp;
+ kmem_free(lessp, sizeof (*lessp));
+ }
+ return (ret);
+}
+
static void
fsd_remove_cb(void *arg, fsh_handle_t handle)
{
_NOTE(ARGUNUSED(handle));
@@ -322,11 +345,11 @@
* It is expected that fsd_lock is being held.
*
* Returns 0 on success and non-zero if hook limit exceeded.
*/
static int
-fsd_install_disturber(vfs_t *vfsp, fsd_t *fsd)
+fsd_disturber_install(vfs_t *vfsp, fsd_t *fsd)
{
fsd_int_t *fsdi;
ASSERT(MUTEX_HELD(&fsd_lock));
@@ -350,11 +373,12 @@
(void) memcpy(&fsdi->fsdi_param, fsd,
sizeof (fsdi->fsdi_param));
rw_init(&fsdi->fsdi_lock, NULL, RW_DRIVER, NULL);
hook.arg = fsdi;
- hook.read = fsd_hook_read;
+ hook.pre_read = fsd_hook_pre_read;
+ hook.post_read = fsd_hook_post_read;
hook.remove_cb = fsd_remove_cb;
/*
* It is safe to do so, because none of the hooks installed
* by fsd uses fsdi_handle nor the fsd_list.
@@ -370,11 +394,11 @@
}
return (0);
}
static int
-fsd_remove_disturber(vfs_t *vfsp)
+fsd_disturber_remove(vfs_t *vfsp)
{
fsd_int_t *fsdi;
ASSERT(MUTEX_HELD(&fsd_lock));
@@ -400,19 +424,19 @@
return (0);
}
static void
-fsd_callback_mount(vfs_t *vfsp, void *arg)
+fsd_mount_callback(vfs_t *vfsp, void *arg)
{
_NOTE(ARGUNUSED(arg));
int error = 0;
mutex_enter(&fsd_lock);
if (fsd_omni_param != NULL)
- error = fsd_install_disturber(vfsp, fsd_omni_param);
+ error = fsd_disturber_install(vfsp, fsd_omni_param);
mutex_exit(&fsd_lock);
if (error != 0) {
refstr_t *mntref;
@@ -421,11 +445,64 @@
refstr_value(mntref));
refstr_rele(mntref);
}
}
+/*
+ * Although, we might delete the fsd_free_callback(), it would make the whole
+ * proces less clear. There's a time window between firing free callbacks and
+ * freeing the vfs_t in fsd_disturber_remove() could be called. fsh can
+ * deal with invalid handles (until there is no collision), but we'd like to
+ * have a nice assertion instead.
+ */
static void
+fsd_free_callback(vfs_t *vfsp, void *arg)
+{
+ _NOTE(ARGUNUSED(arg));
+
+ fsd_int_t *fsdi;
+
+ mutex_enter(&fsd_lock);
+ for (fsdi = list_head(&fsd_list); fsdi != NULL;
+ fsdi = list_next(&fsd_list, fsdi)) {
+ if (fsdi->fsdi_vfsp == vfsp) {
+ if (fsdi->fsdi_doomed)
+ continue;
+
+ fsdi->fsdi_doomed = 1;
+ /*
+ * We make such assertion, because fsd_lock is held
+ * and that means that neither fsd_disturber_remove()
+ * nor fsd_remove_cb() has removed this hook in
+ * different thread.
+ */
+ mutex_enter(&fsd_rem_thread_lock);
+ fsd_rem_thread = curthread;
+ mutex_exit(&fsd_rem_thread_lock);
+
+ ASSERT(fsh_hook_remove(fsdi->fsdi_handle) == 0);
+
+ mutex_enter(&fsd_rem_thread_lock);
+ fsd_rem_thread = NULL;
+ mutex_exit(&fsd_rem_thread_lock);
+
+ /*
+ * Since there is at most one hook installed by fsd,
+ * we break.
+ */
+ break;
+ }
+ }
+ /*
+ * We can't write ASSERT(fsdi != NULL) because it is possible that
+ * there was a concurrent call to fsd_disturber_remove() or
+ * fsd_detach().
+ */
+ mutex_exit(&fsd_lock);
+}
+
+static void
fsd_enable()
{
mutex_enter(&fsd_lock);
fsd_enabled = 1;
mutex_exit(&fsd_lock);
@@ -459,19 +536,20 @@
return (DDI_FAILURE);
fsd_devi = dip;
ddi_report_dev(fsd_devi);
list_create(&fsd_list, sizeof (fsd_int_t),
- offsetof(fsd_int_t, fsdi_next));
+ offsetof(fsd_int_t, fsdi_node));
fsd_rand_seed = gethrtime();
mutex_init(&fsd_lock, NULL, MUTEX_DRIVER, NULL);
mutex_init(&fsd_rem_thread_lock, NULL, MUTEX_DRIVER, NULL);
cv_init(&fsd_cv_empty, NULL, CV_DRIVER, NULL);
- cb.fshc_mount = fsd_callback_mount;
+ cb.fshc_mount = fsd_mount_callback;
+ cb.fshc_free = fsd_free_callback;
cb.fshc_arg = fsd_omni_param;
fsd_cb_handle = fsh_callback_install(&cb);
if (fsd_cb_handle == -1) {
/* Cleanup */
list_destroy(&fsd_list);
@@ -508,13 +586,38 @@
return (DDI_FAILURE);
ddi_remove_minor_node(dip, NULL);
fsd_devi = NULL;
+ /*
+ * 1. Remove the hooks.
+ * 2. Remove the callbacks.
+ *
+ * This order has to be preserved, because of the fact that
+ * fsd_free_callback() is the last stop before a vfs_t is destroyed.
+ * Without it, this might happen:
+ * vfs_free() fsd_detach()
+ * 1. Handle for the hook is
+ * invalidated.
+ * 2. Fired fsd_remove_cb().
+ * 3. fsd_remove_cb() hasn't yet fsd_lock is acquired.
+ * acquired the fsd_lock.
+ * 4 Waiting for fsd_lock. That ASSERT(fsh_hook_remove(..) == 0);
+ * means that the hook hasn't failed, because the handle is
+ * been removed from fsd_hooks already invalid.
+ * fsd_hooks yet.
+ *
+ * The ASSERT() here is nice and without a good reason, we don't want
+ * to get rid of it.
+ */
mutex_enter(&fsd_lock);
+ /*
+ * After we set fsd_detaching to 1, hook remove callback (fsd_remove_cb)
+ * won't try to remove entries from fsd_list.
+ */
fsd_detaching = 1;
- while ((fsdi = list_remove_head(&fsd_list)) != NULL)
+ while ((fsdi = list_remove_head(&fsd_list)) != NULL) {
if (fsdi->fsdi_doomed == 0) {
fsdi->fsdi_doomed = 1;
mutex_enter(&fsd_rem_thread_lock);
fsd_rem_thread = curthread;
@@ -528,10 +631,11 @@
mutex_enter(&fsd_rem_thread_lock);
fsd_rem_thread = NULL;
mutex_exit(&fsd_rem_thread_lock);
}
+ }
while (fsd_list_count > 0)
cv_wait(&fsd_cv_empty, &fsd_lock);
mutex_exit(&fsd_lock);
cv_destroy(&fsd_cv_empty);
@@ -628,11 +732,11 @@
*rvalp = EBADFD;
return (0);
}
mutex_enter(&fsd_lock);
- rv = fsd_install_disturber(file->f_vnode->v_vfsp, &dis.fsdd_param);
+ rv = fsd_disturber_install(file->f_vnode->v_vfsp, &dis.fsdd_param);
mutex_exit(&fsd_lock);
releasef((int)dis.fsdd_mnt);
if (rv != 0)
@@ -781,11 +885,11 @@
*rvalp = EBADFD;
return (0);
}
mutex_enter(&fsd_lock);
- *rvalp = fsd_remove_disturber(file->f_vnode->v_vfsp);
+ *rvalp = fsd_disturber_remove(file->f_vnode->v_vfsp);
releasef((int)fd);
mutex_exit(&fsd_lock);
return (0);
}
@@ -821,11 +925,17 @@
int *rvalp)
{
_NOTE(ARGUNUSED(dev));
_NOTE(ARGUNUSED(credp));
- if (!fsd_enabled && cmd != FSD_ENABLE) {
+ int enabled;
+
+ mutex_enter(&fsd_lock);
+ enabled = fsd_enabled;
+ mutex_exit(&fsd_lock);
+
+ if (!enabled && cmd != FSD_ENABLE) {
*rvalp = ENOTACTIVE;
return (0);
}
switch (cmd) {