From ffbf0e57c2ad9e72ab437c7982f443926cb26325 Mon Sep 17 00:00:00 2001 From: Cyril Plisko Date: Tue, 13 Aug 2013 15:15:36 +0300 Subject: [PATCH] Kstat to use private lock by default While porting Illumos #3537 I found that ks_lock member of kstat_t structure is different between Illumos and SPL. It is a pointer to the kmutex_t in Illumos, but the mutex lock itself in SPL. Apparently Illumos kstat API allows consumer to override the lock if required. With SPL implementation it is not possible anymore. Things were alright until the first attempt to actually override the lock. Porting of Illumos #3537 introduced such code for the first time. In order to provide the Solaris/Illumos like functionality we: 1. convert ks_lock to "kmutex_t *ks_lock" 2. create a new field "kmutex_t ks_private_lock" 3. On kstat_create() ks_lock = &ks_private_lock Thus if consumer doesn't care we still have our internal lock in use. If, however, consumer does care she has a chance to set ks_lock to anything else before calling kstat_install(). The rest of the code will use ks_lock regardless of its origin. Signed-off-by: Ned Bass Signed-off-by: Brian Behlendorf Issue #286 --- include/sys/kstat.h | 3 ++- module/spl/spl-kstat.c | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/sys/kstat.h b/include/sys/kstat.h index 6e3c0cdfe..97d8596f7 100644 --- a/include/sys/kstat.h +++ b/include/sys/kstat.h @@ -114,7 +114,8 @@ struct kstat_s { struct proc_dir_entry *ks_proc; /* proc linkage */ kstat_update_t *ks_update; /* dynamic updates */ void *ks_private; /* private data */ - kmutex_t ks_lock; /* kstat data lock */ + kmutex_t ks_private_lock; /* kstat private data lock */ + kmutex_t *ks_lock; /* kstat data lock */ struct list_head ks_list; /* kstat linkage */ kstat_module_t *ks_owner; /* kstat module linkage */ kstat_raw_ops_t ks_raw_ops; /* ops table for raw type */ diff --git a/module/spl/spl-kstat.c b/module/spl/spl-kstat.c index ee65f4a77..7932678d7 100644 --- a/module/spl/spl-kstat.c +++ b/module/spl/spl-kstat.c @@ -314,7 +314,7 @@ kstat_seq_start(struct seq_file *f, loff_t *pos) ASSERT(ksp->ks_magic == KS_MAGIC); SENTRY; - mutex_enter(&ksp->ks_lock); + mutex_enter(ksp->ks_lock); if (ksp->ks_type == KSTAT_TYPE_RAW) { ksp->ks_raw_bufsize = PAGE_SIZE; @@ -352,13 +352,13 @@ kstat_seq_next(struct seq_file *f, void *p, loff_t *pos) static void kstat_seq_stop(struct seq_file *f, void *v) { - kstat_t *ksp = (kstat_t *)f->private; - ASSERT(ksp->ks_magic == KS_MAGIC); + kstat_t *ksp = (kstat_t *)f->private; + ASSERT(ksp->ks_magic == KS_MAGIC); if (ksp->ks_type == KSTAT_TYPE_RAW) vmem_free(ksp->ks_raw_buf, ksp->ks_raw_bufsize); - mutex_exit(&ksp->ks_lock); + mutex_exit(ksp->ks_lock); } static struct seq_operations kstat_seq_ops = { @@ -491,7 +491,8 @@ __kstat_create(const char *ks_module, int ks_instance, const char *ks_name, mutex_exit(&kstat_module_lock); ksp->ks_magic = KS_MAGIC; - mutex_init(&ksp->ks_lock, NULL, MUTEX_DEFAULT, NULL); + mutex_init(&ksp->ks_private_lock, NULL, MUTEX_DEFAULT, NULL); + ksp->ks_lock = &ksp->ks_private_lock; INIT_LIST_HEAD(&ksp->ks_list); ksp->ks_crtime = gethrtime(); @@ -576,7 +577,7 @@ __kstat_install(kstat_t *ksp) list_add_tail(&ksp->ks_list, &module->ksm_kstat_list); - mutex_enter(&ksp->ks_lock); + mutex_enter(ksp->ks_lock); ksp->ks_owner = module; ksp->ks_proc = proc_create_data(ksp->ks_name, 0644, module->ksm_proc, &proc_kstat_operations, (void *)ksp); @@ -585,7 +586,7 @@ __kstat_install(kstat_t *ksp) if (list_empty(&module->ksm_kstat_list)) kstat_delete_module(module); } - mutex_exit(&ksp->ks_lock); + mutex_exit(ksp->ks_lock); out: mutex_exit(&kstat_module_lock); } @@ -611,7 +612,8 @@ __kstat_delete(kstat_t *ksp) if (!(ksp->ks_flags & KSTAT_FLAG_VIRTUAL)) kmem_free(ksp->ks_data, ksp->ks_data_size); - mutex_destroy(&ksp->ks_lock); + ksp->ks_lock = NULL; + mutex_destroy(&ksp->ks_private_lock); kmem_free(ksp, sizeof(*ksp)); return;