6.2 amd64 panic: lockmgr: thread 0xffffff009f9fd000, not exclusive lock holder 0xffffff003961c000 unlocking

[Guy Helmer]

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