Re: [PATCH libdrm 1/2] amdgpu: prevent an integer wraparound of cpu_map_count

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



You and I discussed this extensively internally a while ago. It's expected and correct behavior. Mesa doesn't unmap some buffers and never will.

Marek

On Wed, Oct 24, 2018 at 3:45 AM Christian König <ckoenig.leichtzumerken@xxxxxxxxx> wrote:
That looks really ugly to me. Mapping the same BO so often is illegal
and should be handled as error.

Otherwise we will never be able to cleanly recover from a GPU lockup
with lost state by reloading the client library.

Christian.

Am 23.10.18 um 21:07 schrieb Marek Olšák:
> From: Marek Olšák <marek.olsak@xxxxxxx>
>
> ---
>   amdgpu/amdgpu_bo.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index c0f42e81..81f8a5f7 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -22,20 +22,21 @@
>    *
>    */
>   
>   #include <stdlib.h>
>   #include <stdio.h>
>   #include <stdint.h>
>   #include <string.h>
>   #include <errno.h>
>   #include <fcntl.h>
>   #include <unistd.h>
> +#include <limits.h>
>   #include <sys/ioctl.h>
>   #include <sys/mman.h>
>   #include <sys/time.h>
>   
>   #include "libdrm_macros.h"
>   #include "xf86drm.h"
>   #include "amdgpu_drm.h"
>   #include "amdgpu_internal.h"
>   #include "util_math.h"
>   
> @@ -442,21 +443,29 @@ drm_public int amdgpu_bo_cpu_map(amdgpu_bo_handle bo, void **cpu)
>   {
>       union drm_amdgpu_gem_mmap args;
>       void *ptr;
>       int r;
>   
>       pthread_mutex_lock(&bo->cpu_access_mutex);
>   
>       if (bo->cpu_ptr) {
>               /* already mapped */
>               assert(bo->cpu_map_count > 0);
> -             bo->cpu_map_count++;
> +
> +             /* If the counter has already reached INT_MAX, don't increment
> +              * it and assume that the buffer will be mapped indefinitely.
> +              * The buffer is pretty unlikely to get unmapped by the user
> +              * at this point.
> +              */
> +             if (bo->cpu_map_count != INT_MAX)
> +                     bo->cpu_map_count++;
> +
>               *cpu = bo->cpu_ptr;
>               pthread_mutex_unlock(&bo->cpu_access_mutex);
>               return 0;
>       }
>   
>       assert(bo->cpu_map_count == 0);
>   
>       memset(&args, 0, sizeof(args));
>   
>       /* Query the buffer address (args.addr_ptr).
> @@ -492,21 +501,27 @@ drm_public int amdgpu_bo_cpu_unmap(amdgpu_bo_handle bo)
>   
>       pthread_mutex_lock(&bo->cpu_access_mutex);
>       assert(bo->cpu_map_count >= 0);
>   
>       if (bo->cpu_map_count == 0) {
>               /* not mapped */
>               pthread_mutex_unlock(&bo->cpu_access_mutex);
>               return -EINVAL;
>       }
>   
> -     bo->cpu_map_count--;
> +     /* If the counter has already reached INT_MAX, don't decrement it.
> +      * This is because amdgpu_bo_cpu_map doesn't increment it past
> +      * INT_MAX.
> +      */
> +     if (bo->cpu_map_count != INT_MAX)
> +             bo->cpu_map_count--;
> +
>       if (bo->cpu_map_count > 0) {
>               /* mapped multiple times */
>               pthread_mutex_unlock(&bo->cpu_access_mutex);
>               return 0;
>       }
>   
>       r = drm_munmap(bo->cpu_ptr, bo->alloc_size) == 0 ? 0 : -errno;
>       bo->cpu_ptr = NULL;
>       pthread_mutex_unlock(&bo->cpu_access_mutex);
>       return r;

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux