kern/120483: [patch] NTFS filesystem locking changes
Scot Hetzel
swhetzel at gmail.com
Sat Feb 9 20:30:01 UTC 2008
>Number: 120483
>Category: kern
>Synopsis: [patch] NTFS filesystem locking changes
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sat Feb 09 20:30:01 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator: Scot Hetzel
>Release: 8.0-CURRENT
>Organization:
>Environment:
FreeBSD hp010 8.0-CURRENT FreeBSD 8.0-CURRENT #0: Sat Feb 9 00:52:06 CST 2008 root at hp010:/usr/src/8x/sys-lstat/amd64/compile/DV8135NR amd64
>Description:
After the lockmgr changes on Jan 08 23:49 UTC 2008, when a NTFS filesystem is mounted a panic will occur:
panic: System call lstat returning with 1 locks held
cpuid = 0
KDB: enter: panic
[thread ; pid 1240 tid 10031]
stopped at kdb_enter+0x3d: movq $0,0x41b048(%rip)
db> show alllocks
db> show locks
db> bt
tracing pid 1240 tid 10031 td 0xffffff001c1ad360
kdb_enter() at kdb_enter+0x3d
panic() at panic+0x176
syscalls() at syscalls+0x66d
Xfast_syscalls() at Xfast_syscalls+0xab
--- syscall (0, FreeBSD ELF64, nosys), rip = 0x8009e87ec, rsp=
0x72ec50, rbp = 0x72ed28 ---
even though the NTFS filesystem wasn't being accessed at the time.
Through the help of Attilio Rao (who had made the changes to lockmgr), and several other users experiencing the problem, we were able to isolate the problem to the NTFS filesystem as the cause of the problem.
>How-To-Repeat:
There are several ways to cause the panic:
Running PREFIX/etc/cvsup/update.sh from a non-NTFS filesystem
Running find on a non-NTFS filesystem (cd /usr/ports ; find . -print) or on the NTFS filesystem.
>Fix:
I looked at the NetBSD NTFS implementation and noticed that they had replaced the lockmgr locking with mutex locking.
After porting the lockmgr -> mutex related changes to FreeBSD, I can no longer cause these panics to occur with a mounted NTFS filesystem after applying the attached patch.
Patch attached with submission follows:
Index: ntfs_ihash.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/ntfs/ntfs_ihash.c,v
retrieving revision 1.23
diff -u -r1.23 ntfs_ihash.c
--- ntfs_ihash.c 13 Nov 2007 19:34:06 -0000 1.23
+++ ntfs_ihash.c 9 Feb 2008 18:55:01 -0000
@@ -40,6 +40,7 @@
#include <sys/malloc.h>
#include <sys/mount.h>
#include <sys/mutex.h>
+#include <sys/condvar.h>
#include <fs/ntfs/ntfs.h>
#include <fs/ntfs/ntfs_inode.h>
@@ -54,7 +55,7 @@
static u_long ntfs_nthash; /* size of hash table - 1 */
#define NTNOHASH(device, inum) (&ntfs_nthashtbl[(minor(device) + (inum)) & ntfs_nthash])
static struct mtx ntfs_nthash_mtx;
-struct lock ntfs_hashlock;
+struct mtx ntfs_hashlock_mtx;
/*
* Initialize inode hash table.
@@ -62,9 +63,9 @@
void
ntfs_nthashinit()
{
- lockinit(&ntfs_hashlock, PINOD, "ntfs_nthashlock", 0, 0);
- ntfs_nthashtbl = hashinit(desiredvnodes, M_NTFSNTHASH, &ntfs_nthash);
+ mtx_init(&ntfs_hashlock_mtx, "ntfs hashlock", NULL, MTX_DEF);
mtx_init(&ntfs_nthash_mtx, "ntfs nthash", NULL, MTX_DEF);
+ ntfs_nthashtbl = hashinit(desiredvnodes, M_NTFSNTHASH, &ntfs_nthash);
}
/*
@@ -74,7 +75,7 @@
ntfs_nthashdestroy(void)
{
hashdestroy(ntfs_nthashtbl, M_NTFSNTHASH, ntfs_nthash);
- lockdestroy(&ntfs_hashlock);
+ mtx_destroy(&ntfs_hashlock_mtx);
mtx_destroy(&ntfs_nthash_mtx);
}
Index: ntfs_ihash.h
===================================================================
RCS file: /home/ncvs/src/sys/fs/ntfs/ntfs_ihash.h,v
retrieving revision 1.8
diff -u -r1.8 ntfs_ihash.h
--- ntfs_ihash.h 16 Jun 2004 09:47:04 -0000 1.8
+++ ntfs_ihash.h 9 Feb 2008 18:55:01 -0000
@@ -28,7 +28,7 @@
* $FreeBSD: src/sys/fs/ntfs/ntfs_ihash.h,v 1.8 2004/06/16 09:47:04 phk Exp $
*/
-extern struct lock ntfs_hashlock;
+extern struct mtx ntfs_hashlock_mtx;
void ntfs_nthashinit(void);
void ntfs_nthashdestroy(void);
struct ntnode *ntfs_nthashlookup(struct cdev *, ino_t);
Index: ntfs_inode.h
===================================================================
RCS file: /home/ncvs/src/sys/fs/ntfs/ntfs_inode.h,v
retrieving revision 1.12
diff -u -r1.12 ntfs_inode.h
--- ntfs_inode.h 16 Jun 2004 09:47:04 -0000 1.12
+++ ntfs_inode.h 9 Feb 2008 18:55:01 -0000
@@ -53,9 +53,10 @@
u_int32_t i_flag;
/* locking */
- struct lock i_lock;
+ struct cv i_lock;
struct mtx i_interlock;
int i_usecount;
+ int i_busy;
LIST_HEAD(,fnode) i_fnlist;
LIST_HEAD(,ntvattr) i_valist;
Index: ntfs_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/ntfs/ntfs_subr.c,v
retrieving revision 1.44
diff -u -r1.44 ntfs_subr.c
--- ntfs_subr.c 24 Jan 2008 12:34:26 -0000 1.44
+++ ntfs_subr.c 9 Feb 2008 19:17:07 -0000
@@ -32,6 +32,7 @@
#include <sys/types.h>
#include <sys/systm.h>
#include <sys/namei.h>
+#include <sys/proc.h>
#include <sys/kernel.h>
#include <sys/vnode.h>
#include <sys/mount.h>
@@ -41,6 +42,8 @@
#include <sys/malloc.h>
#include <sys/lock.h>
#include <sys/iconv.h>
+#include <sys/condvar.h>
+#include <sys/sx.h>
/* #define NTFS_DEBUG 1 */
#include <fs/ntfs/ntfs.h>
@@ -65,7 +68,7 @@
* ntfs mount, freed upon last ntfs umount */
static wchar *ntfs_toupper_tab;
#define NTFS_TOUPPER(ch) (ntfs_toupper_tab[(ch)])
-static struct lock ntfs_toupper_lock;
+static struct sx ntfs_toupper_lock_sx;
static signed int ntfs_toupper_usecount;
struct iconv_functions *ntfs_iconv = NULL;
@@ -358,7 +361,11 @@
mtx_lock(&ip->i_interlock);
ip->i_usecount++;
- lockmgr(&ip->i_lock, LK_EXCLUSIVE | LK_INTERLOCK, &ip->i_interlock);
+ while (ip->i_busy != 0) {
+ cv_wait(&ip->i_lock, &ip->i_interlock);
+ }
+ ip->i_busy = 1;
+ mtx_unlock(&ip->i_interlock);
return 0;
}
@@ -380,21 +387,29 @@
dprintf(("ntfs_ntlookup: looking for ntnode %d\n", ino));
- do {
- ip = ntfs_nthashlookup(ntmp->ntm_devvp->v_rdev, ino);
- if (ip != NULL) {
- ntfs_ntget(ip);
- dprintf(("ntfs_ntlookup: ntnode %d: %p, usecount: %d\n",
- ino, ip, ip->i_usecount));
- *ipp = ip;
- return (0);
- }
- } while (lockmgr(&ntfs_hashlock, LK_EXCLUSIVE | LK_SLEEPFAIL, NULL));
+ *ipp = ntfs_nthashlookup(ntmp->ntm_devvp->v_rdev, ino);
+ if (*ipp != NULL) {
+ ntfs_ntget(*ipp);
+ dprintf(("ntfs_ntlookup: ntnode %d: %p, usecount: %d\n",
+ ino, ipp, (*ipp)->i_usecount));
+ return (0);
+ }
MALLOC(ip, struct ntnode *, sizeof(struct ntnode), M_NTFSNTNODE,
M_WAITOK | M_ZERO);
ddprintf(("ntfs_ntlookup: allocating ntnode: %d: %p\n", ino, ip));
+ mtx_lock(&ntfs_hashlock_mtx);
+ *ipp = ntfs_nthashlookup(ntmp->ntm_devvp->v_rdev, ino);
+ if (*ipp != NULL) {
+ mtx_unlock(&ntfs_hashlock_mtx);
+ ntfs_ntget(*ipp);
+ FREE(ip, M_NTFSNTNODE);
+ dprintf(("ntfs_ntlookup: ntnode %d: %p, usecount: %d\n",
+ ino, ipp, (*ipp)->i_usecount));
+ return (0);
+ }
+
/* Generic initialization */
ip->i_devvp = ntmp->ntm_devvp;
ip->i_dev = ntmp->ntm_devvp->v_rdev;
@@ -405,13 +420,13 @@
VREF(ip->i_devvp);
/* init lock and lock the newborn ntnode */
- lockinit(&ip->i_lock, PINOD, "ntnode", 0, LK_EXCLUSIVE);
+ cv_init(&ip->i_lock, "ntfslk");
mtx_init(&ip->i_interlock, "ntnode interlock", NULL, MTX_DEF);
ntfs_ntget(ip);
ntfs_nthashins(ip);
- lockmgr(&ntfs_hashlock, LK_RELEASE, NULL);
+ mtx_unlock(&ntfs_hashlock_mtx);
*ipp = ip;
@@ -446,27 +461,28 @@
}
#endif
- if (ip->i_usecount > 0) {
- lockmgr(&ip->i_lock, LK_RELEASE|LK_INTERLOCK, &ip->i_interlock);
- return;
- }
-
- dprintf(("ntfs_ntput: deallocating ntnode: %d\n", ip->i_number));
-
- if (LIST_FIRST(&ip->i_fnlist))
- panic("ntfs_ntput: ntnode has fnodes\n");
-
- ntfs_nthashrem(ip);
+ ip->i_busy = 0;
+ cv_signal(&ip->i_lock);
+ mtx_unlock(&ip->i_interlock);
- while ((vap = LIST_FIRST(&ip->i_valist)) != NULL) {
- LIST_REMOVE(vap,va_list);
- ntfs_freentvattr(vap);
+ if (ip->i_usecount == 0) {
+ dprintf(("ntfs_ntput: deallocating ntnode: %d\n",
+ ip->i_number));
+
+ if (LIST_FIRST(&ip->i_fnlist))
+ panic("ntfs_ntput: ntnode has fnodes\n");
+
+ ntfs_nthashrem(ip);
+
+ while ((vap = LIST_FIRST(&ip->i_valist)) != NULL) {
+ LIST_REMOVE(vap,va_list);
+ ntfs_freentvattr(vap);
+ }
+ mtx_destroy(&ip->i_interlock);
+ cv_destroy(&ip->i_lock);
+ vrele(ip->i_devvp);
+ FREE(ip, M_NTFSNTNODE);
}
- mtx_unlock(&ip->i_interlock);
- mtx_destroy(&ip->i_interlock);
- lockdestroy(&ip->i_lock);
- vrele(ip->i_devvp);
- FREE(ip, M_NTFSNTNODE);
}
/*
@@ -1955,7 +1971,7 @@
ntfs_toupper_init()
{
ntfs_toupper_tab = (wchar *) NULL;
- lockinit(&ntfs_toupper_lock, PVFS, "ntfs_toupper", 0, 0);
+ sx_init(&ntfs_toupper_lock_sx, "ntfs toupper lock");
ntfs_toupper_usecount = 0;
}
@@ -1963,7 +1979,7 @@
ntfs_toupper_destroy(void)
{
- lockdestroy(&ntfs_toupper_lock);
+ sx_destroy(&ntfs_toupper_lock_sx);
}
/*
@@ -1979,7 +1995,7 @@
struct vnode *vp;
/* get exclusive access */
- lockmgr(&ntfs_toupper_lock, LK_EXCLUSIVE, NULL);
+ sx_xlock(&ntfs_toupper_lock_sx);
/* only read the translation data from a file if it hasn't been
* read already */
@@ -2002,7 +2018,7 @@
out:
ntfs_toupper_usecount++;
- lockmgr(&ntfs_toupper_lock, LK_RELEASE, NULL);
+ sx_xunlock(&ntfs_toupper_lock_sx);
return (error);
}
@@ -2014,7 +2030,7 @@
ntfs_toupper_unuse()
{
/* get exclusive access */
- lockmgr(&ntfs_toupper_lock, LK_EXCLUSIVE, NULL);
+ sx_xlock(&ntfs_toupper_lock_sx);
ntfs_toupper_usecount--;
if (ntfs_toupper_usecount == 0) {
@@ -2029,7 +2045,7 @@
#endif
/* release the lock */
- lockmgr(&ntfs_toupper_lock, LK_RELEASE, NULL);
+ sx_xunlock(&ntfs_toupper_lock_sx);
}
int
Index: ntfs_vfsops.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/ntfs/ntfs_vfsops.c,v
retrieving revision 1.92
diff -u -r1.92 ntfs_vfsops.c
--- ntfs_vfsops.c 13 Jan 2008 14:44:04 -0000 1.92
+++ ntfs_vfsops.c 9 Feb 2008 18:57:52 -0000
@@ -43,6 +43,7 @@
#include <sys/malloc.h>
#include <sys/stat.h>
#include <sys/systm.h>
+#include <sys/condvar.h>
#include <geom/geom.h>
#include <geom/geom_vfs.h>
Index: ntfs_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/ntfs/ntfs_vnops.c,v
retrieving revision 1.62
diff -u -r1.62 ntfs_vnops.c
--- ntfs_vnops.c 13 Jan 2008 14:44:04 -0000 1.62
+++ ntfs_vnops.c 9 Feb 2008 18:59:35 -0000
@@ -48,6 +48,7 @@
#include <sys/bio.h>
#include <sys/buf.h>
#include <sys/dirent.h>
+#include <sys/condvar.h>
#include <vm/vm.h>
#include <vm/vm_param.h>
>Release-Note:
>Audit-Trail:
>Unformatted:
More information about the freebsd-bugs
mailing list