On 11/30/2012 09:05 AM, Maarten Lankhorst wrote:
> Just use the return error from ttm_mem_evict_first instead.
>
> Changes since v1:
> - Add warning if list is not empty, nothing else we can do here.
Marten, when this function is called, all cross-device reservers have
been shut out with the TTM lock or whatever similar
mechanism is in place. It's a critical function that must succeed,
unless there is a hardware failure to evict.
As mentioned in the comments on the previous patch, ttm_bo_evict_first()
may return 0 (OK) if it failed to reclaim the
trylock in cleanup_refs_and_unlock(), with the assumption that another
process will destroy the bo anyway
(possibly at a later time which we know nothing about). The lru list
needs to be empty when this function returns.
This means we must either keep the while(list_empty) or perhaps better,
retry instead of WARN if list_empty() is detected at the end of list.
/Thomas
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 8ab23ae..9028327 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1323,25 +1323,25 @@ static int ttm_bo_force_list_clean(struct
> ttm_bo_device *bdev,
> struct ttm_bo_global *glob = bdev->glob;
> int ret;
>
> - /*
> - * Can't use standard list traversal since we're unlocking.
> - */
> -
> - spin_lock(&glob->lru_lock);
> - while (!list_empty(&man->lru)) {
> - spin_unlock(&glob->lru_lock);
> + do {
> ret = ttm_mem_evict_first(bdev, mem_type, false, false);
> - if (ret) {
> - if (allow_errors) {
> - return ret;
> - } else {
> - pr_err("Cleanup eviction failed\n");
> - }
> + if (ret && ret != -EBUSY && !allow_errors) {
> + pr_err("Cleanup eviction failed with %i\n", ret);
> + ret = 0;
> }
> + } while (!ret);
> +
> + if (likely(ret == -EBUSY)) {
> + /*
> + * lru list should be empty, verify this is the case.
> + */
> spin_lock(&glob->lru_lock);
> + WARN_ON(!list_empty(&man->lru));
> + if (list_empty(&man->lru) || !allow_errors)
> + ret = 0;
> + spin_unlock(&glob->lru_lock);
> }
> - spin_unlock(&glob->lru_lock);
> - return 0;
> + return ret;
> }
>
> int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type)
