Jerome Glisse wrote: > On Wed, Feb 24, 2010 at 01:37:42PM +0100, Thomas Hellstrom wrote: > >> Jerome Glisse wrote: >> >>> On Tue, Feb 23, 2010 at 02:05:50PM +0100, Thomas Hellstrom wrote: >>> >>>> Jerome Glisse wrote: >>>> >>>>> On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote: >>>>> >>>>>> Jerome Glisse wrote: >>>>>> >>>>>>> On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote: >>>>>>> >>>>>>>> Jerome Glisse wrote: >>>>>>>> >>>>>>>>> Thomas i think i addressed your concern here, the ttm_bo_validate >>>>>>>>> didn't needed a new argument or i did not understand what was >>>>>>>>> necessary beside no_wait. In this patchset we check the value >>>>>>>>> of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS >>>>>>>>> we return VM_FAULT_NOPAGE. >>>>>>>>> >>>>>>>> Well, if we from the fault callback call any function that might >>>>>>>> call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure >>>>>>>> that we never wait, but return -EBUSY all the way back to the >>>>>>>> fault function. Such a case may be ttm_bo_validate that calls >>>>>>>> ttm_bo_evict_first, or something causing a swapout... >>>>>>>> ttm_bo_validate currently doesn't have that functionality, >>>>>>>> because @no_wait just means don't wait for GPU. >>>>>>>> >>>>>>> What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't >>>>>>> use mutex or kernel code path that might sleep ? >>>>>>> >>>>>> I mean waiting while reserving a bo. (If another thread has the bo >>>>>> reserved). >>>>>> >>>>>> >>>>>>> After a new review i don't think we ever wait for the GPU with the >>>>>>> current >>>>>>> patch and as far as i can tell we will return EBUSY or ERESTART all the >>>>>>> way up. >>>>>>> >>>>>>> Cheers, >>>>>>> Jerome >>>>>>> >>>>>> If there is *no* code path trying to reserve a bo or create a >>>>>> user-space visible object from within the fault handler, it should >>>>>> be ok. >>>>>> >>>>>> /Thomas >>>>>> >>>>>> >>>>>> >>>>> Did a new review again here is the call chain : >>>>> ttm_bo_move_buffer >>>>> ttm_bo_mem_space >>>>> ttm_bo_mem_force_space >>>>> ttm_mem_evict_first >>>>> ttm_bo_reserve_locked (no_wait = true) >>>>> >>>> Here ttm_mem_evict_fist may wait for unreserve IIRC (the -EBUSY >>>> return from ttm_bo_reserve_locked) is not propagated back. >>>> >>> The code is not straightforward but if no_wait is true the >>> -EBUSY of ttm_bo_reserve_locked will be propagated back. >>> >> The point is that we don't want to set no_wait to true, because it's >> OK to wait for GPU. We want to add an extra argument >> no_wait_reserve. >> >> /Thomas >> >> > > My patchset doesn't change the code there, no_wait will be true > only if it's true as argument of ttm_bo_validate from the ttm > fault callback in the driver. > >
But you did call ttm_bo_move_buffer from the fault callback? Given the above, it's illegal to call it with no_wait set to false, and undesirable to call it with no_wait set to true. /Thomas > Cheers, > Jerome > ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
