From 9490c148359332d797e4fc250812bd7a5fd131b1 Mon Sep 17 00:00:00 2001 From: behlendo Date: Tue, 11 Mar 2008 20:54:40 +0000 Subject: [PATCH] Apply fix from bug239 for rwlock deadlock. Update check.sh script to take V=1 env var so you can run it verbosely as follows if your chasing something: sudo make check V=1 Add new kobj api and needed regression tests to allow reading of files from within the kernel. Normally thats not something I support but the spa layer needs the support for its config file. Add some more missing stub headers git-svn-id: https://outreach.scidac.gov/svn/spl/trunk@38 7e1ea52c-4ff2-0310-8f11-9dd32ca42a1c --- include/sys/cred.h | 2 +- include/sys/kidmap.h | 4 + include/sys/kobj.h | 29 +++++++ include/sys/rwlock.h | 71 ++++++++++++++++- include/sys/sid.h | 4 + include/sys/sysmacros.h | 1 - include/sys/utsname.h | 8 ++ include/sys/vfs.h | 4 + modules/spl/Makefile.in | 1 + modules/spl/spl-kobj.c | 75 ++++++++++++++++++ modules/splat/Makefile.in | 1 + modules/splat/splat-ctl.c | 2 + modules/splat/splat-internal.h | 4 + modules/splat/splat-kobj.c | 137 +++++++++++++++++++++++++++++++++ scripts/check.sh | 9 ++- 15 files changed, 347 insertions(+), 5 deletions(-) create mode 100644 include/sys/kidmap.h create mode 100644 include/sys/kobj.h create mode 100644 include/sys/sid.h create mode 100644 include/sys/utsname.h create mode 100644 modules/spl/spl-kobj.c create mode 100644 modules/splat/splat-kobj.c diff --git a/include/sys/cred.h b/include/sys/cred.h index 401f3130af..5ed233b0ba 100644 --- a/include/sys/cred.h +++ b/include/sys/cred.h @@ -6,7 +6,7 @@ extern "C" { #endif #include -#include +#include /* XXX - Portions commented out because we really just want to have the type * defined and the contents aren't nearly so important at the moment. */ diff --git a/include/sys/kidmap.h b/include/sys/kidmap.h new file mode 100644 index 0000000000..d1c8d913fb --- /dev/null +++ b/include/sys/kidmap.h @@ -0,0 +1,4 @@ +#ifndef _SPL_KIDMAP_H +#define _SPL_KIDMAP_H + +#endif /* SPL_KIDMAP_H */ diff --git a/include/sys/kobj.h b/include/sys/kobj.h new file mode 100644 index 0000000000..306fcdcfc6 --- /dev/null +++ b/include/sys/kobj.h @@ -0,0 +1,29 @@ +#ifndef _SPL_KOBJ_H +#define _SPL_KOBJ_H + +#ifdef __cplusplus +extern "C" { +#endif + +#include +#include +#include +#include + +typedef struct _buf { + struct file *fp; +} _buf_t; + +extern void *rootdir; + +extern struct _buf *kobj_open_file(const char *name); +extern void kobj_close_file(struct _buf *file); +extern int kobj_read_file(struct _buf *file, char *buf, + unsigned size, unsigned off); +extern int kobj_get_filesize(struct _buf *file, uint64_t *size); + +#ifdef __cplusplus +} +#endif + +#endif /* SPL_KOBJ_H */ diff --git a/include/sys/rwlock.h b/include/sys/rwlock.h index 4498e95626..ecee079483 100644 --- a/include/sys/rwlock.h +++ b/include/sys/rwlock.h @@ -36,6 +36,65 @@ typedef struct { struct task_struct *rw_owner; /* holder of the write lock */ } krwlock_t; +#ifdef CONFIG_RWSEM_GENERIC_SPINLOCK +struct rwsem_waiter { + struct list_head list; + struct task_struct *task; + unsigned int flags; +#define RWSEM_WAITING_FOR_READ 0x00000001 +#define RWSEM_WAITING_FOR_WRITE 0x00000002 +}; + +/* + * wake a single writer + */ +static inline struct rw_semaphore * +__rwsem_wake_one_writer_locked(struct rw_semaphore *sem) +{ + struct rwsem_waiter *waiter; + struct task_struct *tsk; + + sem->activity = -1; + + waiter = list_entry(sem->wait_list.next, struct rwsem_waiter, list); + list_del(&waiter->list); + + tsk = waiter->task; + smp_mb(); + waiter->task = NULL; + wake_up_process(tsk); + put_task_struct(tsk); + return sem; +} + +/* + * release a read lock on the semaphore + */ +static void fastcall +__up_read_locked(struct rw_semaphore *sem) +{ + if (--sem->activity == 0 && !list_empty(&sem->wait_list)) + sem = __rwsem_wake_one_writer_locked(sem); +} + +/* + * trylock for writing -- returns 1 if successful, 0 if contention + */ +static int fastcall +__down_write_trylock_locked(struct rw_semaphore *sem) +{ + int ret = 0; + + if (sem->activity == 0 && list_empty(&sem->wait_list)) { + /* granted */ + sem->activity = -1; + ret = 1; + } + + return ret; +} +#endif + extern int __rw_read_held(krwlock_t *rwlp); extern int __rw_write_held(krwlock_t *rwlp); extern int __rw_lock_held(krwlock_t *rwlp); @@ -168,7 +227,7 @@ rw_downgrade(krwlock_t *rwlp) static __inline__ int rw_tryupgrade(krwlock_t *rwlp) { - int result; + int result = 0; BUG_ON(rwlp->rw_magic != RW_MAGIC); spin_lock(&rwlp->rw_sem.wait_lock); @@ -197,6 +256,15 @@ rw_tryupgrade(krwlock_t *rwlp) return 0; } +#ifdef CONFIG_RWSEM_GENERIC_SPINLOCK + /* Here it should be safe to drop the + * read lock and reacquire it for writing since + * we know there are no waiters */ + __up_read_locked(&rwlp->rw_sem); + + /* returns 1 if success, 0 if contention */ + result = __down_write_trylock_locked(&rwlp->rw_sem); +#else /* Here it should be safe to drop the * read lock and reacquire it for writing since * we know there are no waiters */ @@ -204,6 +272,7 @@ rw_tryupgrade(krwlock_t *rwlp) /* returns 1 if success, 0 if contention */ result = down_write_trylock(&rwlp->rw_sem); +#endif /* Check if upgrade failed. Should not ever happen * if we got to this point */ diff --git a/include/sys/sid.h b/include/sys/sid.h new file mode 100644 index 0000000000..937a71ea81 --- /dev/null +++ b/include/sys/sid.h @@ -0,0 +1,4 @@ +#ifndef _SPL_SID_H +#define _SPL_SID_H + +#endif /* SPL_SID_H */ diff --git a/include/sys/sysmacros.h b/include/sys/sysmacros.h index 3f559bd0ba..218f595676 100644 --- a/include/sys/sysmacros.h +++ b/include/sys/sysmacros.h @@ -135,7 +135,6 @@ extern int highbit(unsigned long i); #define makedevice(maj,min) makedev(maj,min) #define zone_dataset_visible(x, y) (1) #define INGLOBALZONE(z) (1) -#define utsname system_utsname /* XXX - Borrowed from zfs project libsolcompat/include/sys/sysmacros.h */ /* common macros */ diff --git a/include/sys/utsname.h b/include/sys/utsname.h new file mode 100644 index 0000000000..7b1563d4bf --- /dev/null +++ b/include/sys/utsname.h @@ -0,0 +1,8 @@ +#ifndef _SPL_UTSNAME_H +#define _SPL_UTSNAME_H + +#include + +#define utsname system_utsname + +#endif /* SPL_UTSNAME_H */ diff --git a/include/sys/vfs.h b/include/sys/vfs.h index 134f4b6bf4..6bc0a42ae4 100644 --- a/include/sys/vfs.h +++ b/include/sys/vfs.h @@ -1,4 +1,8 @@ #ifndef _SPL_ZFS_H #define _SPL_ZFS_H +typedef struct vfs_s { + int foo; +} vfs_t; + #endif /* SPL_ZFS_H */ diff --git a/modules/spl/Makefile.in b/modules/spl/Makefile.in index 78ebdfa0cb..1005d100b8 100644 --- a/modules/spl/Makefile.in +++ b/modules/spl/Makefile.in @@ -16,6 +16,7 @@ spl-objs += spl-rwlock.o spl-objs += spl-vnode.o spl-objs += spl-err.o spl-objs += spl-time.o +spl-objs += spl-kobj.o spl-objs += spl-generic.o splmodule := spl.ko diff --git a/modules/spl/spl-kobj.c b/modules/spl/spl-kobj.c new file mode 100644 index 0000000000..ce0625b9f1 --- /dev/null +++ b/modules/spl/spl-kobj.c @@ -0,0 +1,75 @@ +#include +#include "config.h" + +void *rootdir = NULL; +EXPORT_SYMBOL(rootdir); + +struct _buf * +kobj_open_file(const char *name) +{ + struct _buf *file; + struct file *fp; + + fp = filp_open(name, O_RDONLY, 0644); + if (IS_ERR(fp)) + return ((_buf_t *)-1UL); + + file = kmem_zalloc(sizeof(_buf_t), KM_SLEEP); + file->fp = fp; + + return file; +} /* kobj_open_file() */ +EXPORT_SYMBOL(kobj_open_file); + +void +kobj_close_file(struct _buf *file) +{ + filp_close(file->fp, 0); + kmem_free(file, sizeof(_buf_t)); + + return; +} /* kobj_close_file() */ +EXPORT_SYMBOL(kobj_close_file); + +int +kobj_read_file(struct _buf *file, char *buf, unsigned size, unsigned off) +{ + loff_t offset = off; + mm_segment_t saved_fs; + int rc; + + if (!file || !file->fp) + return -EINVAL; + + if (!file->fp->f_op || !file->fp->f_op->read) + return -ENOSYS; + + /* Writable user data segment must be briefly increased for this + * process so we can use the user space read call paths to write + * in to memory allocated by the kernel. */ + saved_fs = get_fs(); + set_fs(get_ds()); + rc = file->fp->f_op->read(file->fp, buf, size, &offset); + set_fs(saved_fs); + + return rc; +} /* kobj_read_file() */ +EXPORT_SYMBOL(kobj_read_file); + +int +kobj_get_filesize(struct _buf *file, uint64_t *size) +{ + struct kstat stat; + int rc; + + if (!file || !file->fp || !size) + return -EINVAL; + + rc = vfs_getattr(file->fp->f_vfsmnt, file->fp->f_dentry, &stat); + if (rc) + return rc; + + *size = stat.size; + return rc; +} /* kobj_get_filesize() */ +EXPORT_SYMBOL(kobj_get_filesize); diff --git a/modules/splat/Makefile.in b/modules/splat/Makefile.in index 29cec45818..0c7526f197 100644 --- a/modules/splat/Makefile.in +++ b/modules/splat/Makefile.in @@ -21,6 +21,7 @@ splat-objs += splat-condvar.o splat-objs += splat-thread.o splat-objs += splat-rwlock.o splat-objs += splat-time.o +splat-objs += splat-kobj.o splatmodule := splat.ko splatmoduledir := @kmoduledir@/kernel/lib/ diff --git a/modules/splat/splat-ctl.c b/modules/splat/splat-ctl.c index 968acc82af..693277c5cf 100644 --- a/modules/splat/splat-ctl.c +++ b/modules/splat/splat-ctl.c @@ -591,6 +591,7 @@ splat_init(void) SPLAT_SUBSYSTEM_INIT(thread); SPLAT_SUBSYSTEM_INIT(rwlock); SPLAT_SUBSYSTEM_INIT(time); + SPLAT_SUBSYSTEM_INIT(kobj); dev = MKDEV(SPLAT_MAJOR, 0); if ((rc = register_chrdev_region(dev, SPLAT_MINORS, "splatctl"))) @@ -652,6 +653,7 @@ splat_fini(void) cdev_del(&splat_cdev); unregister_chrdev_region(dev, SPLAT_MINORS); + SPLAT_SUBSYSTEM_FINI(kobj); SPLAT_SUBSYSTEM_FINI(time); SPLAT_SUBSYSTEM_FINI(rwlock); SPLAT_SUBSYSTEM_FINI(thread); diff --git a/modules/splat/splat-internal.h b/modules/splat/splat-internal.h index 061a9d0181..b8e8032904 100644 --- a/modules/splat/splat-internal.h +++ b/modules/splat/splat-internal.h @@ -32,6 +32,7 @@ #include #include #include +#include #include "splat-ctl.h" @@ -169,6 +170,7 @@ splat_subsystem_t * splat_rwlock_init(void); splat_subsystem_t * splat_taskq_init(void); splat_subsystem_t * splat_thread_init(void); splat_subsystem_t * splat_time_init(void); +splat_subsystem_t * splat_kobj_init(void); void splat_condvar_fini(splat_subsystem_t *); void splat_kmem_fini(splat_subsystem_t *); @@ -178,6 +180,7 @@ void splat_rwlock_fini(splat_subsystem_t *); void splat_taskq_fini(splat_subsystem_t *); void splat_thread_fini(splat_subsystem_t *); void splat_time_fini(splat_subsystem_t *); +void splat_kobj_fini(splat_subsystem_t *); int splat_condvar_id(void); int splat_kmem_id(void); @@ -187,5 +190,6 @@ int splat_rwlock_id(void); int splat_taskq_id(void); int splat_thread_id(void); int splat_time_id(void); +int splat_kobj_id(void); #endif /* _SPLAT_INTERNAL_H */ diff --git a/modules/splat/splat-kobj.c b/modules/splat/splat-kobj.c new file mode 100644 index 0000000000..91f591f462 --- /dev/null +++ b/modules/splat/splat-kobj.c @@ -0,0 +1,137 @@ +#include "splat-internal.h" + +#define SPLAT_SUBSYSTEM_KOBJ 0x0900 +#define SPLAT_KOBJ_NAME "kobj" +#define SPLAT_KOBJ_DESC "Kernel File Tests" + +#define SPLAT_KOBJ_TEST1_ID 0x0901 +#define SPLAT_KOBJ_TEST1_NAME "kobj1" +#define SPLAT_KOBJ_TEST1_DESC "File Open/Close Test" + +#define SPLAT_KOBJ_TEST2_ID 0x0902 +#define SPLAT_KOBJ_TEST2_NAME "kobj2" +#define SPLAT_KOBJ_TEST2_DESC "File Size/Read Test" + +#define SPLAT_KOBJ_TEST_FILE "/etc/fstab" + +static int +splat_kobj_test1(struct file *file, void *arg) +{ + struct _buf *f; + + f = kobj_open_file(SPLAT_KOBJ_TEST_FILE); + if (f == (struct _buf *)-1) { + splat_vprint(file, SPLAT_KOBJ_TEST1_NAME, "Failed to open " + "test file: %s\n", SPLAT_KOBJ_TEST_FILE); + return -ENOENT; + } + + kobj_close_file(f); + splat_vprint(file, SPLAT_KOBJ_TEST1_NAME, "Successfully opened and " + "closed test file: %s\n", SPLAT_KOBJ_TEST_FILE); + + return 0; +} /* splat_kobj_test1() */ + +static int +splat_kobj_test2(struct file *file, void *arg) +{ + struct _buf *f; + char *buf; + uint64_t size; + int rc; + + f = kobj_open_file(SPLAT_KOBJ_TEST_FILE); + if (f == (struct _buf *)-1) { + splat_vprint(file, SPLAT_KOBJ_TEST2_NAME, "Failed to open " + "test file: %s\n", SPLAT_KOBJ_TEST_FILE); + return -ENOENT; + } + + rc = kobj_get_filesize(f, &size); + if (rc) { + splat_vprint(file, SPLAT_KOBJ_TEST2_NAME, "Failed stat of " + "test file: %s (%d)\n", SPLAT_KOBJ_TEST_FILE, rc); + goto out; + } + + buf = kmalloc(size, GFP_KERNEL); + if (!buf) { + rc = -ENOMEM; + splat_vprint(file, SPLAT_KOBJ_TEST2_NAME, "Failed to alloc " + "%lld bytes for tmp buffer (%d)\n", size, rc); + goto out; + } + + rc = kobj_read_file(f, buf, size, 0); + if (rc < 0) { + splat_vprint(file, SPLAT_KOBJ_TEST2_NAME, "Failed read of " + "test file: %s (%d)\n", SPLAT_KOBJ_TEST_FILE, rc); + goto out2; + } + + /* Validate we read as many bytes as expected based on the stat. This + * isn't a perfect test since we didn't create the file however it is + * pretty unlikely there are garbage characters in your /etc/fstab */ + if (size != (uint64_t)strlen(buf)) { + rc = EFBIG; + splat_vprint(file, SPLAT_KOBJ_TEST2_NAME, "Stat'ed size " + "(%lld) does not match number of bytes read " + "(%lld)\n", size, (uint64_t)strlen(buf)); + goto out2; + } + + rc = 0; + splat_vprint(file, SPLAT_KOBJ_TEST2_NAME, "\n%s\n", buf); + splat_vprint(file, SPLAT_KOBJ_TEST2_NAME, "Successfully stat'ed " + "and read expected number of bytes (%lld) from test " + "file: %s\n", size, SPLAT_KOBJ_TEST_FILE); +out2: + kfree(buf); +out: + kobj_close_file(f); + + return rc; +} /* splat_kobj_test2() */ + +splat_subsystem_t * +splat_kobj_init(void) +{ + splat_subsystem_t *sub; + + sub = kmalloc(sizeof(*sub), GFP_KERNEL); + if (sub == NULL) + return NULL; + + memset(sub, 0, sizeof(*sub)); + strncpy(sub->desc.name, SPLAT_KOBJ_NAME, SPLAT_NAME_SIZE); + strncpy(sub->desc.desc, SPLAT_KOBJ_DESC, SPLAT_DESC_SIZE); + INIT_LIST_HEAD(&sub->subsystem_list); + INIT_LIST_HEAD(&sub->test_list); + spin_lock_init(&sub->test_lock); + sub->desc.id = SPLAT_SUBSYSTEM_KOBJ; + + SPLAT_TEST_INIT(sub, SPLAT_KOBJ_TEST1_NAME, SPLAT_KOBJ_TEST1_DESC, + SPLAT_KOBJ_TEST1_ID, splat_kobj_test1); + SPLAT_TEST_INIT(sub, SPLAT_KOBJ_TEST2_NAME, SPLAT_KOBJ_TEST2_DESC, + SPLAT_KOBJ_TEST2_ID, splat_kobj_test2); + + return sub; +} /* splat_kobj_init() */ + +void +splat_kobj_fini(splat_subsystem_t *sub) +{ + ASSERT(sub); + + SPLAT_TEST_FINI(sub, SPLAT_KOBJ_TEST2_ID); + SPLAT_TEST_FINI(sub, SPLAT_KOBJ_TEST1_ID); + + kfree(sub); +} /* splat_kobj_fini() */ + +int +splat_kobj_id(void) +{ + return SPLAT_SUBSYSTEM_KOBJ; +} /* splat_kobj_id() */ diff --git a/scripts/check.sh b/scripts/check.sh index d98c670928..5d71236841 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -4,6 +4,7 @@ prog=check.sh spl_module=../modules/spl/spl.ko splat_module=../modules/splat/splat.ko splat_cmd=../cmd/splat +verbose= die() { echo "${prog}: $1" >&2 @@ -14,6 +15,10 @@ warn() { echo "${prog}: $1" >&2 } +if [ -n "$V" ]; then + verbose="-v" +fi + if [ $(id -u) != 0 ]; then die "Must run as root" fi @@ -32,8 +37,8 @@ echo "Loading ${spl_module}" echo "Loading ${splat_module}" /sbin/insmod ${splat_module} || die "Unable to load ${splat_module}" -sleep 5 -$splat_cmd -a +sleep 3 +$splat_cmd -a $verbose echo "Unloading ${splat_module}" /sbin/rmmod ${splat_module} || die "Failed to unload ${splat_module}"