On Tue, 2009-06-09 at 13:25 -0400, John Baldwin wrote: > On Tuesday 09 June 2009 1:00:25 pm Kostik Belousov wrote: > > On Tue, Jun 09, 2009 at 09:45:49AM -0700, Matthew Fleming wrote: > > > > > > > This appears to be an interaction with the recent changes to use > > > > shared vnode locks for writes on ZFS. Hmm, I think it may be ok to > > > > use a shared vnode lock for kevents on vnodes though. The vnode > > > > interlock should be sufficient locking for what little work the kevent > > > > > > > filters do. As a quick hack for now the MNT_SHARED_WRITES() stuff > > > > could avoid using shared locks 'if (!VN_KNLIST_EMPTY(vp))', but I > > > > think the longer term fix is to not use the vnode locks for vnode > > > kevents, but use the interlock instead. > > > > > > I tried (briefly) using the interlock since Isilon's vnode lock is > > > cluster wide (in our 6.1 based code we got away with using Giant). This > > > got me a LOR report on the interlock: > > > > > > /* > > > * kqueue/VFS interaction > > > */ > > > { "kqueue", &lock_class_mtx_sleep }, > > > { "struct mount mtx", &lock_class_mtx_sleep }, > > > { "vnode interlock", &lock_class_mtx_sleep }, > > > { NULL, NULL }, > > > > > > since knote() will take first the list->kl_lock and then the kqueue > > > lock. I didn't spend any time on it, and switched to using the vnode > > > v_lock for my purposes. But someone added that lock ordering (r166421) > > > for a reason. > > > > That was me, I actually looked for the reversed order that was reported > > several times on the list in 6.1-6.2 timeframe. Unfortunately, nothing > > was found. > > > > I noted in the separate letter that read filter for vnodes needs > > shared vnode lock anyway. > > So my current idea is to allow shared vnode locks and use the vnode interlock > in the filt_vfs_* routines to protect access to the kn_* member variables. > > --- //depot/projects/smpng/sys/kern/vfs_subr.c 2009/06/09 15:15:22 > +++ //depot/user/jhb/lock/kern/vfs_subr.c 2009/06/09 17:20:33 > _at__at_ -4103,8 +4103,10 _at__at_ > vfs_knllocked(void *arg) > { > struct vnode *vp = arg; > + int islocked; > > - return (VOP_ISLOCKED(vp) == LK_EXCLUSIVE); > + islocked = VOP_ISLOCKED(vp); > + return (islocked == LK_SHARED || islocked == LK_EXCLUSIVE); > } > > int > _at__at_ -4114,6 +4116,7 _at__at_ > struct knote *kn = ap->a_kn; > struct knlist *knl; > > + ASSERT_VOP_ELOCKED(vp, "vfs_kqfilter"); > switch (kn->kn_filter) { > case EVFILT_READ: > kn->kn_fop = &vfsread_filtops; > _at__at_ -4147,6 +4150,7 _at__at_ > { > struct vnode *vp = (struct vnode *)kn->kn_hook; > > + ASSERT_VOP_ELOCKED(vp, "filt_vfsdetach"); > KASSERT(vp->v_pollinfo != NULL, ("Missing v_pollinfo")); > knlist_remove(&vp->v_pollinfo->vpi_selinfo.si_note, kn, 0); > } > _at__at_ -4157,48 +4161,65 _at__at_ > { > struct vnode *vp = (struct vnode *)kn->kn_hook; > struct vattr va; > + int retval; > > /* > * filesystem is gone, so set the EOF flag and schedule > * the knote for deletion. > */ > if (hint == NOTE_REVOKE) { > + VI_LOCK(vp); > kn->kn_flags |= (EV_EOF | EV_ONESHOT); > + VI_UNLOCK(vp); > return (1); > } > > if (VOP_GETATTR(vp, &va, curthread->td_ucred)) > return (0); > > + VI_LOCK(vp); > kn->kn_data = va.va_size - kn->kn_fp->f_offset; > - return (kn->kn_data != 0); > + retval = (kn->kn_data != 0); > + VI_UNLOCK(vp); > + return (retval); > } > > /*ARGSUSED*/ > static int > filt_vfswrite(struct knote *kn, long hint) > { > + struct vnode *vp = (struct vnode *)kn->kn_hook; > + > /* > * filesystem is gone, so set the EOF flag and schedule > * the knote for deletion. > */ > + VI_LOCK(vp); > if (hint == NOTE_REVOKE) > kn->kn_flags |= (EV_EOF | EV_ONESHOT); > > kn->kn_data = 0; > + VI_UNLOCK(vp); > return (1); > } > > static int > filt_vfsvnode(struct knote *kn, long hint) > { > + struct vnode *vp = (struct vnode *)kn->kn_hook; > + int retval; > + > + VI_LOCK(vp); > if (kn->kn_sfflags & hint) > kn->kn_fflags |= hint; > if (hint == NOTE_REVOKE) { > kn->kn_flags |= EV_EOF; > + VI_UNLOCK(vp); > return (1); > } > - return (kn->kn_fflags != 0); > + retval = (kn->kn_fflags != 0); > + VI_UNLOCK(vp); > + return (retval); > } > > int Getting this panic with the patch. I have core files if I can get you anything. (kgdb) #0 doadump () at pcpu.h:223 #1 0xffffffff80590503 in boot (howto=260) at /usr/src/sys/kern/kern_shutdown.c:419 #2 0xffffffff8059098c in panic (fmt=Variable "fmt" is not available. ) at /usr/src/sys/kern/kern_shutdown.c:575 #3 0xffffffff80560115 in knlist_add (knl=0xffffff0067190718, kn=0xffffff00671be528, islocked=0) at /usr/src/sys/kern/kern_event.c:1656 #4 0xffffffff8061bc14 in vfs_kqfilter (ap=Variable "ap" is not available. ) at /usr/src/sys/kern/vfs_subr.c:4140 #5 0xffffffff808c1e97 in VOP_KQFILTER_APV (vop=0xffffffff80bc4580, a=0xffffff803e661820) at vnode_if.c:1141 #6 0xffffffff8062a241 in vn_kqfilter (fp=0xffffff0056f5f820, kn=0xffffff00671be528) at vnode_if.h:500 #7 0xffffffff80562084 in kqueue_register (kq=0xffffff0005adf000, kev=0xffffff803e661920, td=0xffffff004054d720, waitok=1) at /usr/src/sys/kern/kern_event.c:968 #8 0xffffffff805623f2 in kern_kevent (td=0xffffff004054d720, fd=Variable "fd" is not available. ) at /usr/src/sys/kern/kern_event.c:721 #9 0xffffffff80562e10 in kevent (td=0xffffff004054d720, uap=0xffffff803e661c00) at /usr/src/sys/kern/kern_event.c:642 #10 0xffffffff80877e5d in syscall (frame=0xffffff803e661c90) at /usr/src/sys/amd64/amd64/trap.c:984 #11 0xffffffff808527d0 in Xfast_syscall () at /usr/src/sys/amd64/amd64/exception.S:364 #12 0x0000000800e2672c in ?? () Previous frame inner to this frame (corrupt stack?) (kgdb) robert. > > -- > John Baldwin > _______________________________________________ > freebsd-current_at_freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org" -- Robert Noland <rnoland_at_FreeBSD.org> FreeBSD
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:49 UTC