<div dir="ltr"><div class="gmail_quote"><div>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.</div><div><br></div><div>Marek<br></div><div dir="ltr"><br></div><div dir="ltr">On Wed, Oct 24, 2018 at 3:45 AM Christian König <<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank">ckoenig.leichtzumerken@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">That looks really ugly to me. Mapping the same BO so often is illegal <br>
and should be handled as error.<br>
<br>
Otherwise we will never be able to cleanly recover from a GPU lockup <br>
with lost state by reloading the client library.<br>
<br>
Christian.<br>
<br>
Am 23.10.18 um 21:07 schrieb Marek Olšák:<br>
> From: Marek Olšák <<a href="mailto:marek.olsak@amd.com" target="_blank">marek.olsak@amd.com</a>><br>
><br>
> ---<br>
> amdgpu/amdgpu_bo.c | 19 +++++++++++++++++--<br>
> 1 file changed, 17 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c<br>
> index c0f42e81..81f8a5f7 100644<br>
> --- a/amdgpu/amdgpu_bo.c<br>
> +++ b/amdgpu/amdgpu_bo.c<br>
> @@ -22,20 +22,21 @@<br>
> *<br>
> */<br>
> <br>
> #include <stdlib.h><br>
> #include <stdio.h><br>
> #include <stdint.h><br>
> #include <string.h><br>
> #include <errno.h><br>
> #include <fcntl.h><br>
> #include <unistd.h><br>
> +#include <limits.h><br>
> #include <sys/ioctl.h><br>
> #include <sys/mman.h><br>
> #include <sys/time.h><br>
> <br>
> #include "libdrm_macros.h"<br>
> #include "xf86drm.h"<br>
> #include "amdgpu_drm.h"<br>
> #include "amdgpu_internal.h"<br>
> #include "util_math.h"<br>
> <br>
> @@ -442,21 +443,29 @@ drm_public int amdgpu_bo_cpu_map(amdgpu_bo_handle bo, void **cpu)<br>
> {<br>
> union drm_amdgpu_gem_mmap args;<br>
> void *ptr;<br>
> int r;<br>
> <br>
> pthread_mutex_lock(&bo->cpu_access_mutex);<br>
> <br>
> if (bo->cpu_ptr) {<br>
> /* already mapped */<br>
> assert(bo->cpu_map_count > 0);<br>
> - bo->cpu_map_count++;<br>
> +<br>
> + /* If the counter has already reached INT_MAX, don't increment<br>
> + * it and assume that the buffer will be mapped indefinitely.<br>
> + * The buffer is pretty unlikely to get unmapped by the user<br>
> + * at this point.<br>
> + */<br>
> + if (bo->cpu_map_count != INT_MAX)<br>
> + bo->cpu_map_count++;<br>
> +<br>
> *cpu = bo->cpu_ptr;<br>
> pthread_mutex_unlock(&bo->cpu_access_mutex);<br>
> return 0;<br>
> }<br>
> <br>
> assert(bo->cpu_map_count == 0);<br>
> <br>
> memset(&args, 0, sizeof(args));<br>
> <br>
> /* Query the buffer address (args.addr_ptr).<br>
> @@ -492,21 +501,27 @@ drm_public int amdgpu_bo_cpu_unmap(amdgpu_bo_handle bo)<br>
> <br>
> pthread_mutex_lock(&bo->cpu_access_mutex);<br>
> assert(bo->cpu_map_count >= 0);<br>
> <br>
> if (bo->cpu_map_count == 0) {<br>
> /* not mapped */<br>
> pthread_mutex_unlock(&bo->cpu_access_mutex);<br>
> return -EINVAL;<br>
> }<br>
> <br>
> - bo->cpu_map_count--;<br>
> + /* If the counter has already reached INT_MAX, don't decrement it.<br>
> + * This is because amdgpu_bo_cpu_map doesn't increment it past<br>
> + * INT_MAX.<br>
> + */<br>
> + if (bo->cpu_map_count != INT_MAX)<br>
> + bo->cpu_map_count--;<br>
> +<br>
> if (bo->cpu_map_count > 0) {<br>
> /* mapped multiple times */<br>
> pthread_mutex_unlock(&bo->cpu_access_mutex);<br>
> return 0;<br>
> }<br>
> <br>
> r = drm_munmap(bo->cpu_ptr, bo->alloc_size) == 0 ? 0 : -errno;<br>
> bo->cpu_ptr = NULL;<br>
> pthread_mutex_unlock(&bo->cpu_access_mutex);<br>
> return r;<br>
<br>
</blockquote></div></div>