Re: radeon_cp_texture: page fault with non-sleepable locks held

On 11/08/10 07:16, Kostik Belousov wrote:
> On Mon, Nov 08, 2010 at 03:06:01PM +0200, Andriy Gapon wrote:
>    
>> on 08/11/2010 14:04 Kostik Belousov said the following:
>>      
>>> On Mon, Nov 08, 2010 at 01:50:25PM +0200, Andriy Gapon wrote:
>>>        
>>>> on 05/11/2010 09:27 Andriy Gapon said the following:
>>>>          
>>>>> Kernel page fault with the following non-sleepable locks held:
>>>>> exclusive sleep mutex drmdev (drmdev) r = 0 (0xffffff0001b968a0) locked _at_
>>>>> /usr/src/sys/dev/drm/drm_drv.c:791
>>>>> KDB: stack backtrace:
>>>>> db_trace_self_wrapper() at 0xffffffff801b8afa = db_trace_self_wrapper+0x2a
>>>>> kdb_backtrace() at 0xffffffff803a7afa = kdb_backtrace+0x3a
>>>>> _witness_debugger() at 0xffffffff803bd49c = _witness_debugger+0x2c
>>>>> witness_warn() at 0xffffffff803bed32 = witness_warn+0x322
>>>>> trap() at 0xffffffff8054639f = trap+0x39f
>>>>>            
>> Kostik,
>>
>> a tangential question - do you think that it would make sense to put a check
>> like the above (in trap) into copyin/copyout (but non-fatal), so that we can
>> catch such situations pro-actively (without having to wait for a page fault to
>> actually happen)?
>>      
> uiomove() already contains
> 	WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL,
> 	    "Calling uiomove()");
> at the start.
>
> For the copyin/out routines, that are implemented in assembler for
> most (all ?) architectures, this seems to be overkill, IMHO.
>    

The other issue is that this can be a legal thing to do. If you have 
taken care to wire the userland buffers ahead of time, there is no 
problem copying copyin()/copyout() with sleepable locks held. The sysctl 
code does this. As such, you can't check for problems by panicing if 
sleepable locks are held.
-Nathan