System call munmap returning with the following locks
held: Giant
John Baldwin
jhb at freebsd.org
Thu Jan 19 10:15:24 PST 2006
On Thursday 19 January 2006 08:02, John Baldwin wrote:
> On Wednesday 18 January 2006 08:31 pm, Suleiman Souhlal wrote:
> > Hi,
> >
> > John Baldwin wrote:
> > > I sent this to you on IRC, but for the archives, here's a possible fix.
> > > It looks like vm_object_deallocate() never unlocks Giant if it locks
> > > it, and the leak would only happen if mpsafevfs=0 or you are using a
> > > non-safe filesystem:
> >
> > The real problem is that vm_object_deallocate() doesn't expect the
> > object's type to change if it sees it's a vnode, when it's not holding
> > the object lock:
> > /*
> > * In general, the object should be locked when working with
> > * its type. In this case, in order to maintain proper lock
> > * ordering, an exception is possible because a vnode-backed
> > * object never changes its type.
> > */
> > vfslocked = 0;
> > if (object->type == OBJT_VNODE) {
> > struct vnode *vp = (struct vnode *) object->handle;
> > vfslocked = VFS_LOCK_GIANT(vp->v_mount);
> > }
> > VM_OBJECT_LOCK(object);
> > if (object->type == OBJT_VNODE) {
> > vm_object_vndeallocate(object);
> > VFS_UNLOCK_GIANT(vfslocked);
> > return;
> > }
> >
> > The comment is actually wrong, and the object's type can change to
> > OBJT_DEAD when the corresponing vnode gets freed, so maybe you might
> > want to change it.
>
> Well, that's not the cause of Kris' panic at all (the function really is
> not ever dropping Giant). If the object does change to OBJT_DEAD after
> Giant is acquired then some of the MPASS()'s I added might fail I think.
> I'm not sure if that's all that has to be done to fix the problem you are
> concerned about.
Actually, if the object's type isn't guaranteed to be stable by teh caller
somehow, then the VFS_LOCK_GIANT part itself is racey. Really fixing it
would be something ugly like this:
vfslocked = 0;
restart:
VM_OBJECT_LOCK(object);
if (object->type == OBJT_VNODE) {
struct vnode *vp = (struct vnode *)object->handle;
if (VFS_NEEDSGIANT(vp->v_mount) {
VM_OBJECT_UNLOCK(object);
mtx_lock(&Giant);
vfslocked = 1;
goto restart;
} else
VFS_UNLOCK_GIANT(vfslocked);
} else
VFS_UNLOCK_GIANT(vfslocked);
...
Are you really sure the object's type can change or does the caller of
vm_object_deallocate() hold some sort of reference or what not that prevents
the type from changing?
--
John Baldwin <jhb at FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
More information about the freebsd-current
mailing list