[PATCH 1/2] drm/ttm: fix re-init of global structures

[Karol Herbst]

On Tue, Apr 16, 2019 at 12:53 PM Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> When a driver unloads without unloading TTM we don't correctly
> clear the global structures leading to errors on re-init.
>
> Next step should probably be to remove the global structures and
> kobjs all together, but this is tricky since we need to maintain
> backward compatibility.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c     | 10 +++++-----
>  drivers/gpu/drm/ttm/ttm_memory.c |  5 +++--
>  include/drm/ttm/ttm_bo_driver.h  |  1 -
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 41d07faa2eae..2845fceb2fbd 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -49,9 +49,8 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj);
>   * ttm_global_mutex - protecting the global BO state
>   */
>  DEFINE_MUTEX(ttm_global_mutex);
> -struct ttm_bo_global ttm_bo_glob = {
> -       .use_count = 0
> -};
> +unsigned ttm_bo_glob_use_count;
> +struct ttm_bo_global ttm_bo_glob;
>
>  static struct attribute ttm_bo_count = {
>         .name = "bo_count",
> @@ -1531,12 +1530,13 @@ static void ttm_bo_global_release(void)
>         struct ttm_bo_global *glob = &ttm_bo_glob;
>
>         mutex_lock(&ttm_global_mutex);
> -       if (--glob->use_count > 0)
> +       if (--ttm_bo_glob_use_count > 0)
>                 goto out;
>
>         kobject_del(&glob->kobj);
>         kobject_put(&glob->kobj);
>         ttm_mem_global_release(&ttm_mem_glob);
> +       memset(glob, 0, sizeof(*glob));
>  out:
>         mutex_unlock(&ttm_global_mutex);
>  }
> @@ -1548,7 +1548,7 @@ static int ttm_bo_global_init(void)
>         unsigned i;
>
>         mutex_lock(&ttm_global_mutex);
> -       if (++glob->use_count > 1)
> +       if (++ttm_bo_glob_use_count > 1)
>                 goto out;
>
>         ret = ttm_mem_global_init(&ttm_mem_glob);
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
> index 699fed9e08ee..8617958b7ae6 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -461,8 +461,8 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
>
>  void ttm_mem_global_release(struct ttm_mem_global *glob)
>  {
> -       unsigned int i;
>         struct ttm_mem_zone *zone;
> +       unsigned int i;

comment below

>
>         /* let the page allocator first stop the shrink work. */
>         ttm_page_alloc_fini();
> @@ -475,9 +475,10 @@ void ttm_mem_global_release(struct ttm_mem_global *glob)
>                 zone = glob->zones[i];
>                 kobject_del(&zone->kobj);
>                 kobject_put(&zone->kobj);
> -                       }
> +       }

comment below

>         kobject_del(&glob->kobj);
>         kobject_put(&glob->kobj);
> +       memset(glob, 0, sizeof(*glob));
>  }
>
>  static void ttm_check_swapping(struct ttm_mem_global *glob)
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 3db265bf2d4e..c008346c2401 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -420,7 +420,6 @@ extern struct ttm_bo_global {
>         /**
>          * Protected by ttm_global_mutex.
>          */
> -       unsigned int use_count;
>         struct list_head device_list;
>
>         /**
> --
> 2.17.1
>
tested on 5.0.7 and it fixes the crash I've encountered, so:
Tested-by: Karol Herbst <kherbst at redhat.com>

Personally I think we should split formatting and other unrelated
changes out of a patch for stable, but those are still fine changes,
anyway:
Reviewed-by: Karol Herbst <kherbst at redhat.com>

Thanks!