<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 3, 2017 at 9:40 AM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Fri, Apr 28, 2017 at 1:34 AM, Juan A. Suarez Romero <span dir="ltr"><<a href="mailto:jasuarez@igalia.com" target="_blank">jasuarez@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div class="m_-1799825248514011420h5"><div>On Thu, 2017-04-27 at 20:36 -0700, Jason Ekstrand wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Apr 27, 2017 at 9:23 AM, Juan A. Suarez Romero <span dir="ltr"><<a href="mailto:jasuarez@igalia.com" target="_blank">jasuarez@igalia.com</a>></span> wrote:<br><blockquote type="cite"><div class="m_-1799825248514011420m_-2829555526173127935HOEnZb"><div class="m_-1799825248514011420m_-2829555526173127935h5">On Wed, 2017-04-26 at 07:35 -0700, Jason Ekstrand wrote:<br>
> We should only use size_t when referring to sizes of bits of CPU memory.<br>
> Anything on the GPU or just a regular array length should be a type that<br>
> has the same size on both 32 and 64-bit architectures.  For state<br>
> objects, we use a uint32_t because we'll never allocate a piece of<br>
> driver-internal GPU state larger than 2GB (more like 16KB).<br>
> ---<br>
>  src/intel/vulkan/anv_allocator<wbr>.c | 12 ++++++------<br>
>  src/intel/vulkan/anv_gem.c       |  2 +-<br>
>  src/intel/vulkan/anv_gem_stubs<wbr>.c |  2 +-<br>
>  src/intel/vulkan/anv_private.h<wbr>   | 12 ++++++------<br>
>  4 files changed, 14 insertions(+), 14 deletions(-)<br>
><br>
> diff --git a/src/intel/vulkan/anv_allocat<wbr>or.c b/src/intel/vulkan/anv_allocat<wbr>or.c<br>
> index 1c94d1b..2dad400 100644<br>
> --- a/src/intel/vulkan/anv_allocat<wbr>or.c<br>
> +++ b/src/intel/vulkan/anv_allocat<wbr>or.c<br>
> @@ -342,7 +342,7 @@ anv_block_pool_finish(struct anv_block_pool *pool)<br>
>  static uint32_t<br>
>  anv_block_pool_grow(struct anv_block_pool *pool, struct anv_block_state *state)<br>
>  {<br>
> -   size_t size;<br>
> +   uint32_t size;<br>
>     void *map;<br>
>     uint32_t gem_handle;<br>
>     struct anv_mmap_cleanup *cleanup;<br>
> @@ -367,7 +367,7 @@ anv_block_pool_grow(struct anv_block_pool *pool, struct anv_block_state *state)<br>
><br>
>     assert(state == &pool->state || back_used > 0);<br>
><br>
> -   size_t old_size = pool->bo.size;<br>
> +   uint32_t old_size = pool->bo.size;<br>
><br>
>     if (old_size != 0 &&<br>
>         back_used * 2 <= pool->center_bo_offset &&<br>
<br>
<br>
</div></div>While checking this patch, I've notice we have the following comment in<br>
the anv_block_pool_grow()<br>
<br>
   /* We can't have a block pool bigger than 1GB because we use signed<br>
    * 32-bit offsets in the free list and we don't want overflow.  We<br>
    * should never need a block pool bigger than 1GB anyway.<br>
    */<br>
   assert(size <= (1u << 31));<br>
<br>
Is the comment correct? Shouldn't say 2Gb?<br></blockquote><div><br></div><div>1 or 2; I can't remember at the moment.  I thought there was something weird with Android that required us to drop it to 1.<br></div><div> </div></div></div></div></blockquote><div><br></div></div></div><div>If it's 1Gb, I think assertion should be "assert(size <= (1u << 30));"</div></div></blockquote><div><br></div></div></div><div>How about "assert(size <= BLOCK_POOL_MEMFD_SIZE)"?  That seems better than hard-coded things :-)<br></div></div></div></div></blockquote><div><br></div><div>I just sent a new patch to the list which applies at the end of the series to address this.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div class="m_-1799825248514011420h5"><div><br></div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote type="cite">
Other than that,<br>
<br>
Reviewed-by: Juan A. Suarez Romero <<a href="mailto:jasuarez@igalia.com" target="_blank">jasuarez@igalia.com</a>><br>
<div><div class="m_-1799825248514011420m_-2829555526173127935h5"><br>
<br>
> @@ -613,7 +613,7 @@ anv_block_pool_free(struct anv_block_pool *pool, int32_t offset)<br>
><br>
>  static void<br>
>  anv_fixed_size_state_pool_init<wbr>(struct anv_fixed_size_state_pool *pool,<br>
> -                               size_t state_size)<br>
> +                               uint32_t state_size)<br>
>  {<br>
>     /* At least a cache line and must divide the block size. */<br>
>     assert(state_size >= 64 && util_is_power_of_two(state_siz<wbr>e));<br>
> @@ -672,7 +672,7 @@ anv_state_pool_init(struct anv_state_pool *pool,<br>
>  {<br>
>     pool->block_pool = block_pool;<br>
>     for (unsigned i = 0; i < ANV_STATE_BUCKETS; i++) {<br>
> -      size_t size = 1 << (ANV_MIN_STATE_SIZE_LOG2 + i);<br>
> +      uint32_t size = 1 << (ANV_MIN_STATE_SIZE_LOG2 + i);<br>
>        anv_fixed_size_state_pool_init<wbr>(&pool->buckets[i], size);<br>
>     }<br>
>     VG(VALGRIND_CREATE_MEMPOOL(po<wbr>ol, 0, false));<br>
> @@ -686,7 +686,7 @@ anv_state_pool_finish(struct anv_state_pool *pool)<br>
><br>
>  static struct anv_state<br>
>  anv_state_pool_alloc_no_vg(str<wbr>uct anv_state_pool *pool,<br>
> -                           size_t size, size_t align)<br>
> +                           uint32_t size, uint32_t align)<br>
>  {<br>
>     unsigned size_log2 = ilog2_round_up(size < align ? align : size);<br>
>     assert(size_log2 <= ANV_MAX_STATE_SIZE_LOG2);<br>
> @@ -703,7 +703,7 @@ anv_state_pool_alloc_no_vg(str<wbr>uct anv_state_pool *pool,<br>
>  }<br>
><br>
>  struct anv_state<br>
> -anv_state_pool_alloc(struct anv_state_pool *pool, size_t size, size_t align)<br>
> +anv_state_pool_alloc(struct anv_state_pool *pool, uint32_t size, uint32_t align)<br>
>  {<br>
>     if (size == 0)<br>
>        return ANV_STATE_NULL;<br>
> diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c<br>
> index 185086f..4b6ee58 100644<br>
> --- a/src/intel/vulkan/anv_gem.c<br>
> +++ b/src/intel/vulkan/anv_gem.c<br>
> @@ -48,7 +48,7 @@ anv_ioctl(int fd, unsigned long request, void *arg)<br>
>   * Return gem handle, or 0 on failure. Gem handles are never 0.<br>
>   */<br>
>  uint32_t<br>
> -anv_gem_create(struct anv_device *device, size_t size)<br>
> +anv_gem_create(struct anv_device *device, uint64_t size)<br>
>  {<br>
>     struct drm_i915_gem_create gem_create = {<br>
>        .size = size,<br>
> diff --git a/src/intel/vulkan/anv_gem_stu<wbr>bs.c b/src/intel/vulkan/anv_gem_stu<wbr>bs.c<br>
> index a63e96d..8d81eb5 100644<br>
> --- a/src/intel/vulkan/anv_gem_stu<wbr>bs.c<br>
> +++ b/src/intel/vulkan/anv_gem_stu<wbr>bs.c<br>
> @@ -34,7 +34,7 @@ memfd_create(const char *name, unsigned int flags)<br>
>  }<br>
><br>
>  uint32_t<br>
> -anv_gem_create(struct anv_device *device, size_t size)<br>
> +anv_gem_create(struct anv_device *device, uint64_t size)<br>
>  {<br>
>     int fd = memfd_create("fake bo", MFD_CLOEXEC);<br>
>     if (fd == -1)<br>
> diff --git a/src/intel/vulkan/anv_private<wbr>.h b/src/intel/vulkan/anv_private<wbr>.h<br>
> index 216c6eb..c709d39 100644<br>
> --- a/src/intel/vulkan/anv_private<wbr>.h<br>
> +++ b/src/intel/vulkan/anv_private<wbr>.h<br>
> @@ -493,7 +493,7 @@ struct anv_state {<br>
>  #define ANV_STATE_NULL ((struct anv_state) { .alloc_size = 0 })<br>
><br>
>  struct anv_fixed_size_state_pool {<br>
> -   size_t state_size;<br>
> +   uint32_t state_size;<br>
>     union anv_free_list free_list;<br>
>     struct anv_block_state block;<br>
>  };<br>
> @@ -565,7 +565,7 @@ void anv_state_pool_init(struct anv_state_pool *pool,<br>
>                           struct anv_block_pool *block_pool);<br>
>  void anv_state_pool_finish(struct anv_state_pool *pool);<br>
>  struct anv_state anv_state_pool_alloc(struct anv_state_pool *pool,<br>
> -                                      size_t state_size, size_t alignment);<br>
> +                                      uint32_t state_size, uint32_t alignment);<br>
>  void anv_state_pool_free(struct anv_state_pool *pool, struct anv_state state);<br>
>  void anv_state_stream_init(struct anv_state_stream *stream,<br>
>                             struct anv_state_pool *state_pool,<br>
> @@ -752,7 +752,7 @@ VkResult anv_device_wait(struct anv_device *device, struct anv_bo *bo,<br>
>  void* anv_gem_mmap(struct anv_device *device,<br>
>                     uint32_t gem_handle, uint64_t offset, uint64_t size, uint32_t flags);<br>
>  void anv_gem_munmap(void *p, uint64_t size);<br>
> -uint32_t anv_gem_create(struct anv_device *device, size_t size);<br>
> +uint32_t anv_gem_create(struct anv_device *device, uint64_t size);<br>
>  void anv_gem_close(struct anv_device *device, uint32_t gem_handle);<br>
>  uint32_t anv_gem_userptr(struct anv_device *device, void *mem, size_t size);<br>
>  int anv_gem_busy(struct anv_device *device, uint32_t gem_handle);<br>
> @@ -780,8 +780,8 @@ int anv_gem_set_domain(struct anv_device *device, uint32_t gem_handle,<br>
>  VkResult anv_bo_init_new(struct anv_bo *bo, struct anv_device *device, uint64_t size);<br>
><br>
>  struct anv_reloc_list {<br>
> -   size_t                                       num_relocs;<br>
> -   size_t                                       array_length;<br>
> +   uint32_t                                     num_relocs;<br>
> +   uint32_t                                     array_length;<br>
>     struct drm_i915_gem_relocation_entry *       relocs;<br>
>     struct anv_bo **                             reloc_bos;<br>
>  };<br>
> @@ -803,7 +803,7 @@ struct anv_batch_bo {<br>
>     struct anv_bo                                bo;<br>
><br>
>     /* Bytes actually consumed in this batch BO */<br>
> -   size_t                                       length;<br>
> +   uint32_t                                     length;<br>
><br>
>     struct anv_reloc_list                        relocs;<br>
>  };<br>
</div></div>______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div></div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>