Date: Thu, 01 Feb 2007 10:55:18 -0600 From: Guy Helmer <ghelmer@palisadesys.com> To: Kostik Belousov <kostikbel@gmail.com> Cc: freebsd-stable@freebsd.org Subject: Re: 6.2 amd64 panic: lockmgr: thread 0xffffff009f9fd000, not exclusive lock holder 0xffffff003961c000 unlocking Message-ID: <45C21B76.8090301@palisadesys.com> In-Reply-To: <20070201164631.GJ56152@deviant.kiev.zoral.com.ua> References: <45C0C749.1040300@palisadesys.com> <45C20F00.4010900@palisadesys.com> <20070201163826.GI56152@deviant.kiev.zoral.com.ua> <20070201164631.GJ56152@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Kostik Belousov wrote:
> On Thu, Feb 01, 2007 at 06:38:26PM +0200, Kostik Belousov wrote:
>
>> On Thu, Feb 01, 2007 at 10:02:08AM -0600, Guy Helmer wrote:
>> vn_lock with LK_EXCLUSIVE|LK_RETRY flags combination shall not fail. It should
>> return even dead vnodes locked.
>>
>> I suspect that in fact this is race with exec(). Could you reproduce the
>> panic ? And then, with this patch ?
>>
>> Index: fs/procfs/procfs.c
>> ===================================================================
>> RCS file: /usr/local/arch/ncvs/src/sys/fs/procfs/procfs.c,v
>> retrieving revision 1.14
>> diff -u -r1.14 procfs.c
>> --- fs/procfs/procfs.c 5 Jun 2006 16:41:27 -0000 1.14
>> +++ fs/procfs/procfs.c 1 Feb 2007 16:37:43 -0000
>> @@ -69,10 +69,12 @@
>> {
>> char *fullpath = "unknown";
>> char *freepath = NULL;
>> + struct vnode *textvp;
>>
>> - vn_lock(p->p_textvp, LK_EXCLUSIVE | LK_RETRY, td);
>> - vn_fullpath(td, p->p_textvp, &fullpath, &freepath);
>> - VOP_UNLOCK(p->p_textvp, 0, td);
>> + textvp = p->p_textvp;
>> + vn_lock(textvp, LK_EXCLUSIVE | LK_RETRY, td);
>> + vn_fullpath(td, textvp, &fullpath, &freepath);
>> + VOP_UNLOCK(textvp, 0, td);
>> sbuf_printf(sb, "%s", fullpath);
>> if (freepath)
>> free(freepath, M_TEMP);
>>
>
> That patch is incorrect as well: textvp may be reclaimed while waiting for
> lock. Below is better version.
>
> Index: fs/procfs/procfs.c
> ===================================================================
> RCS file: /usr/local/arch/ncvs/src/sys/fs/procfs/procfs.c,v
> retrieving revision 1.14
> diff -u -r1.14 procfs.c
> --- fs/procfs/procfs.c 5 Jun 2006 16:41:27 -0000 1.14
> +++ fs/procfs/procfs.c 1 Feb 2007 16:44:41 -0000
> @@ -69,10 +69,17 @@
> {
> char *fullpath = "unknown";
> char *freepath = NULL;
> + struct vnode *textvp;
> + int err;
>
> - vn_lock(p->p_textvp, LK_EXCLUSIVE | LK_RETRY, td);
> - vn_fullpath(td, p->p_textvp, &fullpath, &freepath);
> - VOP_UNLOCK(p->p_textvp, 0, td);
> + textvp = p->p_textvp;
> + VI_LOCK(textvp);
> + vholdl(textvp);
> + err = vn_lock(textvp, LK_EXCLUSIVE | LK_INTERLOCK, td);
> + vdrop(textvp);
> + if (err)
>
Shouldn't that be "if (!err)"?
> + vn_fullpath(td, textvp, &fullpath, &freepath);
> + VOP_UNLOCK(textvp, 0, td);
> sbuf_printf(sb, "%s", fullpath);
> if (freepath)
> free(freepath, M_TEMP);
>
Thanks for your insight and patch!
Guy
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45C21B76.8090301>