Skip to content

Commit

Permalink
Remove misguided HAVE_MUTEX_OWNER check, take 2
Browse files Browse the repository at this point in the history
It is just plain unsafe to peek inside in-kernel
mutex structure and make assumptions about what kernel
does with those internal fields like owner.

Kernel is all too happy to stop doing the expected things
like tracing lock owner once you load a tainted module
like spl/zfs that is not GPL.

As such you will get instant assertion failures like this:

  VERIFY3(((*(volatile typeof((&((&zo->zo_lock)->m_mutex))->owner) *)&
      ((&((&zo->zo_lock)->m_mutex))->owner))) ==
     ((void *)0)) failed (ffff88030be28500 == (null))
  PANIC at zfs_onexit.c:104:zfs_onexit_destroy()
  Showing stack for process 3626
  CPU: 0 PID: 3626 Comm: mkfs.lustre Tainted: P OE ------------ 3.10.0-debug openzfs#1
  Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
  Call Trace:
  dump_stack+0x19/0x1b
  spl_dumpstack+0x44/0x50 [spl]
  spl_panic+0xbf/0xf0 [spl]
  zfs_onexit_destroy+0x17c/0x280 [zfs]
  zfsdev_release+0x48/0xd0 [zfs]

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Chunwei Chen <[email protected]>
Reviewed-by: Gvozden Neskovic <[email protected]>
Signed-off-by: Oleg Drokin <[email protected]>
Closes openzfs#639
Closes openzfs#632
  • Loading branch information
verygreen authored and behlendorf committed Aug 3, 2017
1 parent 261a315 commit 98cdcb8
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 45 deletions.
30 changes: 0 additions & 30 deletions config/spl-build.m4
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ AC_DEFUN([SPL_AC_CONFIG_KERNEL], [
SPL_AC_USLEEP_RANGE
SPL_AC_KMEM_CACHE_ALLOCFLAGS
SPL_AC_WAIT_ON_BIT
SPL_AC_MUTEX_OWNER
SPL_AC_INODE_LOCK
SPL_AC_GROUP_INFO_GID
SPL_AC_KMEM_CACHE_CREATE_USERCOPY
Expand Down Expand Up @@ -1562,35 +1561,6 @@ AC_DEFUN([SPL_AC_WAIT_ON_BIT], [
])
])

dnl #
dnl # Check whether mutex has owner with task_struct type.
dnl #
dnl # Note that before Linux 3.0, mutex owner is of type thread_info.
dnl #
dnl # Note that in Linux 3.18, the condition for owner is changed from
dnl # defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_SMP) to
dnl # defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_MUTEX_SPIN_ON_OWNER)
dnl #
AC_DEFUN([SPL_AC_MUTEX_OWNER], [
AC_MSG_CHECKING([whether mutex has owner])
tmp_flags="$EXTRA_KCFLAGS"
EXTRA_KCFLAGS="-Werror"
SPL_LINUX_TRY_COMPILE([
#include <linux/mutex.h>
#include <linux/spinlock.h>
],[
DEFINE_MUTEX(m);
struct task_struct *t __attribute__ ((unused));
t = m.owner;
],[
AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_MUTEX_OWNER, 1, [yes])
],[
AC_MSG_RESULT(no)
])
EXTRA_KCFLAGS="$tmp_flags"
])

dnl #
dnl # 4.7 API change
dnl # i_mutex is changed to i_rwsem. Instead of directly using
Expand Down
15 changes: 0 additions & 15 deletions include/sys/mutex.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ typedef enum {
typedef struct {
struct mutex m_mutex;
spinlock_t m_lock; /* used for serializing mutex_exit */
#ifndef HAVE_MUTEX_OWNER
/* only when kernel doesn't have owner */
kthread_t *m_owner;
#endif
#ifdef CONFIG_LOCKDEP
kmutex_type_t m_type;
#endif /* CONFIG_LOCKDEP */
Expand All @@ -54,28 +51,16 @@ typedef struct {
static inline void
spl_mutex_set_owner(kmutex_t *mp)
{
/*
* kernel will handle its owner, so we don't need to do anything if it
* is defined.
*/
#ifndef HAVE_MUTEX_OWNER
mp->m_owner = current;
#endif
}

static inline void
spl_mutex_clear_owner(kmutex_t *mp)
{
#ifndef HAVE_MUTEX_OWNER
mp->m_owner = NULL;
#endif
}

#ifdef HAVE_MUTEX_OWNER
#define mutex_owner(mp) (ACCESS_ONCE(MUTEX(mp)->owner))
#else
#define mutex_owner(mp) (ACCESS_ONCE((mp)->m_owner))
#endif
#define mutex_owned(mp) (mutex_owner(mp) == current)
#define MUTEX_HELD(mp) mutex_owned(mp)
#define MUTEX_NOT_HELD(mp) (!MUTEX_HELD(mp))
Expand Down

0 comments on commit 98cdcb8

Please sign in to comment.