Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Jul 2007 12:59:56 +0800
From:      Ganbold <ganbold@micom.mng.net>
To:        Kostik Belousov <kostikbel@gmail.com>
Cc:        x11@freebsd.org
Subject:   Re: LOR when running google-earth
Message-ID:  <469C4CCC.5000102@micom.mng.net>
In-Reply-To: <20070717042309.GY2200@deviant.kiev.zoral.com.ua>
References:  <469B17BB.9050305@micom.mng.net> <20070716105316.GT2200@deviant.kiev.zoral.com.ua> <469C240B.4060201@micom.mng.net> <20070717042309.GY2200@deviant.kiev.zoral.com.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
Thanks for the patch, Konstantin.
It seems like LOR is gone after the patch.
However I got following when I tried to switch virtual desktops under beryl:

error: [drm:pid1:i915_emit_box] *ERROR* Bad box 64485,100..0,774
error: [drm:pid1:i915_cmdbuffer] *ERROR* i915_dispatch_cmdbuffer failed
error: [drm:pid1:i915_emit_box] *ERROR* Bad box 64485,100..0,774
error: [drm:pid1:i915_cmdbuffer] *ERROR* i915_dispatch_cmdbuffer failed
error: [drm:pid1157:drm_close] *ERROR* can't find authenticator

On shell where I'm running googleearth it shows:

DRM_I830_CMDBUFFER:  -22

and googleearth crashes :(

Maybe above problems are related to beryl/emerald.

Ganbold

Kostik Belousov wrote:
> On Tue, Jul 17, 2007 at 10:06:03AM +0800, Ganbold wrote:
>   
>> Hi,
>>
>> I have new LOR here after applying this patch:
>>
>> lock order reversal: (sleepable after non-sleepable)
>> 1st 0xc3e3e4d8 drm device (drm device) @ 
>> /usr/src/sys/modules/drm/i915/../../../dev/drm/i915_dma.c:675
>> 2nd 0xc42bb784 user map (user map) @ /usr/src/sys/vm/vm_map.c:1818
>> KDB: stack backtrace:
>> db_trace_self_wrapper(c08a24a7,e6386a08,c066801e,c08a4988,c42bb784,...) 
>> at db_trace_self_wrapper+0x26
>> kdb_backtrace(c08a4988,c42bb784,c08bca1a,c08bca1a,c08bc9af,...) at 
>> kdb_backtrace+0x29
>> witness_checkorder(c42bb784,9,c08bc9af,71a,e6386a3c,...) at 
>> witness_checkorder+0x6de
>> _sx_xlock(c42bb784,0,c08bc9af,71a,c3e3e400,...) at _sx_xlock+0x7d
>> _vm_map_lock(c42bb740,c08bc9af,71a,e6386a84,c06677fc,...) at 
>> _vm_map_lock+0x51
>> vm_map_unwire(c42bb740,8570000,8571000,0,e6386b34,...) at vm_map_unwire+0x2d
>> vsunlock(8570df8,30,c450171e,2a3,e6386b0c,...) at vsunlock+0x46
>> i915_cmdbuffer(c4358b00,8018644b,c3e1ae00,3,c3ef0000,...) at 
>> i915_cmdbuffer+0x5c8
>> drm_ioctl(c4358b00,8018644b,c3e1ae00,3,c3ef0000,...) at drm_ioctl+0x357
>> giant_ioctl(c4358b00,8018644b,c3e1ae00,3,c3ef0000,...) at giant_ioctl+0x56
>> devfs_ioctl_f(c4e10678,8018644b,c3e1ae00,c4359a00,c3ef0000,...) at 
>> devfs_ioctl_f+0xc9
>> kern_ioctl(c3ef0000,9,8018644b,c3e1ae00,386c4c,...) at kern_ioctl+0x243
>> ioctl(c3ef0000,e6386cfc,e6386c80,c4027e4a,c3ef0000,...) at ioctl+0x134
>> linux_ioctl_drm(c3ef0000,e6386cfc,c4035c45,a2f,c3ef0000,...) at 
>> linux_ioctl_drm+0x2f
>> linux_ioctl(c3ef0000,e6386cfc,e6386cf8,e6386d1c,c4037dd0,...) at 
>> linux_ioctl+0xca
>> syscall(e6386d38) at syscall+0x2b3
>> Xint0x80_syscall() at Xint0x80_syscall+0x20
>> --- syscall (54, Linux ELF, linux_ioctl), eip = 0x49ea95f4, esp = 
>> 0xbfbfdf0c, ebp = 0xbfbfdf2c ---
>> KDB: enter: witness_checkorder
>> [thread pid 991 tid 100083 ]
>> Stopped at      kdb_enter+0x32: leave
>> db> bt
>> Tracing pid 991 tid 100083 td 0xc3ef0000
>> kdb_enter(c085f570,c42bb784,c08bca1a,c08bca1a,c08bc9af,...) at 
>> kdb_enter+0x32
>> witness_checkorder(c42bb784,9,c08bc9af,71a,e6386a3c,...) at 
>> witness_checkorder+0x6f3
>> _sx_xlock(c42bb784,0,c08bc9af,71a,c3e3e400,...) at _sx_xlock+0x7d
>> _vm_map_lock(c42bb740,c08bc9af,71a,e6386a84,c06677fc,...) at 
>> _vm_map_lock+0x51
>> vm_map_unwire(c42bb740,8570000,8571000,0,e6386b34,...) at vm_map_unwire+0x2d
>> vsunlock(8570df8,30,c450171e,2a3,e6386b0c,...) at vsunlock+0x46
>> i915_cmdbuffer(c4358b00,8018644b,c3e1ae00,3,c3ef0000,...) at 
>> i915_cmdbuffer+0x5c8
>> drm_ioctl(c4358b00,8018644b,c3e1ae00,3,c3ef0000,...) at drm_ioctl+0x357
>> giant_ioctl(c4358b00,8018644b,c3e1ae00,3,c3ef0000,...) at giant_ioctl+0x56
>> devfs_ioctl_f(c4e10678,8018644b,c3e1ae00,c4359a00,c3ef0000,...) at 
>> devfs_ioctl_f+0xc9
>> kern_ioctl(c3ef0000,9,8018644b,c3e1ae00,386c4c,...) at kern_ioctl+0x243
>> ioctl(c3ef0000,e6386cfc,e6386c80,c4027e4a,c3ef0000,...) at ioctl+0x134
>> linux_ioctl_drm(c3ef0000,e6386cfc,c4035c45,a2f,c3ef0000,...) at 
>> linux_ioctl_drm+0x2f
>> linux_ioctl(c3ef0000,e6386cfc,e6386cf8,e6386d1c,c4037dd0,...) at 
>> linux_ioctl+0xca
>> syscall(e6386d38) at syscall+0x2b3
>> Xint0x80_syscall() at Xint0x80_syscall+0x20
>> --- syscall (54, Linux ELF, linux_ioctl), eip = 0x49ea95f4, esp = 
>> 0xbfbfdf0c, ebp = 0xbfbfdf2c ---
>> db>
>>     
> My fault. Updated patch below.
>
> Did you experienced any other bad effects of the patch, aside from the
> LOR above ?
>
>
> diff --git a/sys/dev/drm/i915_dma.c b/sys/dev/drm/i915_dma.c
> index dfaf334..c9dd799 100644
> --- a/sys/dev/drm/i915_dma.c
> +++ b/sys/dev/drm/i915_dma.c
> @@ -367,20 +367,14 @@ static int i915_emit_cmds(drm_device_t * dev, int __user * buffer, int dwords)
>  	for (i = 0; i < dwords;) {
>  		int cmd, sz;
>  
> -	     if (DRM_COPY_FROM_USER_UNCHECKED(&cmd, &buffer[i], sizeof(cmd))) {
> -
> -			return DRM_ERR(EINVAL);
> -	      }
> +		cmd = buffer[i];
>  		if ((sz = validate_cmd(cmd)) == 0 || i + sz > dwords)
>  			return DRM_ERR(EINVAL);
>  
>  		OUT_RING(cmd);
>  
>  		while (++i, --sz) {
> -			if (DRM_COPY_FROM_USER_UNCHECKED(&cmd, &buffer[i],
> -							 sizeof(cmd))) {
> -				return DRM_ERR(EINVAL);
> -			}
> +			cmd = buffer[i];
>  			OUT_RING(cmd);
>  		}
>  	}
> @@ -401,10 +395,7 @@ static int i915_emit_box(drm_device_t * dev,
>  	drm_clip_rect_t box;
>  	RING_LOCALS;
>  
> -	if (DRM_COPY_FROM_USER_UNCHECKED(&box, &boxes[i], sizeof(box))) {
> -		return EFAULT;
> -	}
> -
> +	box = boxes[i];
>  	if (box.y2 <= box.y1 || box.x2 <= box.x1 || box.y2 <= 0 || box.x2 <= 0) {
>  		DRM_ERROR("Bad box %d,%d..%d,%d\n",
>  			  box.x1, box.y1, box.x2, box.y2);
> @@ -604,6 +595,7 @@ static int i915_batchbuffer(DRM_IOCTL_ARGS)
>  	drm_i915_sarea_t *sarea_priv = (drm_i915_sarea_t *)
>  	    dev_priv->sarea_priv;
>  	drm_i915_batchbuffer_t batch;
> +	size_t cliplen;
>  	int ret;
>  
>  	if (!dev_priv->allow_batchbuffer) {
> @@ -619,14 +611,25 @@ static int i915_batchbuffer(DRM_IOCTL_ARGS)
>  
>  	LOCK_TEST_WITH_RETURN(dev, filp);
>  
> +	DRM_UNLOCK();
> +	cliplen = batch.num_cliprects * sizeof(drm_clip_rect_t);	
>  	if (batch.num_cliprects && DRM_VERIFYAREA_READ(batch.cliprects,
> -						       batch.num_cliprects *
> -						       sizeof(drm_clip_rect_t)))
> +						       cliplen)) {
> +		DRM_LOCK();
>  		return DRM_ERR(EFAULT);
> -
> +	}
> +	ret = vslock(batch.cliprects, cliplen);
> +	if (ret) {
> +		DRM_ERROR("Fault wiring cliprects\n");
> +		DRM_LOCK();
> +		return DRM_ERR(EFAULT);
> +	}
> +	DRM_LOCK();
>  	ret = i915_dispatch_batchbuffer(dev, &batch);
> -
>  	sarea_priv->last_dispatch = (int)hw_status[5];
> +	DRM_UNLOCK();
> +	vsunlock(batch.cliprects, cliplen);
> +	DRM_LOCK();
>  	return ret;
>  }
>  
> @@ -638,6 +641,7 @@ static int i915_cmdbuffer(DRM_IOCTL_ARGS)
>  	drm_i915_sarea_t *sarea_priv = (drm_i915_sarea_t *)
>  	    dev_priv->sarea_priv;
>  	drm_i915_cmdbuffer_t cmdbuf;
> +	size_t cliplen;
>  	int ret;
>  
>  	DRM_COPY_FROM_USER_IOCTL(cmdbuf, (drm_i915_cmdbuffer_t __user *) data,
> @@ -648,22 +652,38 @@ static int i915_cmdbuffer(DRM_IOCTL_ARGS)
>  
>  	LOCK_TEST_WITH_RETURN(dev, filp);
>  
> +	DRM_UNLOCK();
> +	cliplen = cmdbuf.num_cliprects * sizeof(drm_clip_rect_t);
>  	if (cmdbuf.num_cliprects &&
> -	    DRM_VERIFYAREA_READ(cmdbuf.cliprects,
> -				cmdbuf.num_cliprects *
> -				sizeof(drm_clip_rect_t))) {
> +	    DRM_VERIFYAREA_READ(cmdbuf.cliprects, cliplen)) {
>  		DRM_ERROR("Fault accessing cliprects\n");
> +		DRM_LOCK();
>  		return DRM_ERR(EFAULT);
>  	}
> -
> -	ret = i915_dispatch_cmdbuffer(dev, &cmdbuf);
> +	ret = vslock(cmdbuf.cliprects, cliplen);
>  	if (ret) {
> -		DRM_ERROR("i915_dispatch_cmdbuffer failed\n");
> -		return ret;
> +		DRM_ERROR("Fault wiring cliprects\n");
> +		DRM_LOCK();
> +		return DRM_ERR(EFAULT);
>  	}
> -
> -	sarea_priv->last_dispatch = (int)hw_status[5];
> -	return 0;
> +	ret = vslock(cmdbuf.buf, cmdbuf.sz);
> +	if (ret) {
> +		vsunlock(cmdbuf.cliprects, cliplen);
> +		DRM_ERROR("Fault wiring cmds\n");
> +		DRM_LOCK();
> +		return DRM_ERR(EFAULT);
> +	}
> +	DRM_LOCK();
> +	ret = i915_dispatch_cmdbuffer(dev, &cmdbuf);
> +	if (ret == 0)
> +		sarea_priv->last_dispatch = (int)hw_status[5];
> +	else
> +		DRM_ERROR("i915_dispatch_cmdbuffer failed\n");
> +	DRM_UNLOCK();
> +	vsunlock(cmdbuf.buf, cmdbuf.sz);
> +	vsunlock(cmdbuf.cliprects, cliplen);
> +	DRM_LOCK();
> +	return (ret);
>  }
>  
>  static int i915_do_cleanup_pageflip(drm_device_t * dev)
>   


-- 
There is no statute of limitations on stupidity.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?469C4CCC.5000102>