On Wed, Feb 15, 2017 at 1:54 PM, Chad Versace <chadvers...@chromium.org>
wrote:
> On Wed 15 Feb 2017, Emil Velikov wrote:
> > On 14 February 2017 at 21:02, Chad Versace <chadvers...@chromium.org>
> wrote:
> > > On Tue 14 Feb 2017, Kenneth Graunke wrote:
> > >> On Tuesday, February 14, 2017 12:38:45 PM PST Chad Versace wrote:
> > >> > On Tue 14 Feb 2017, Matt Turner wrote:
> > >> >
> > >> >
> > >> > > static bool
> > >> > > -anv_get_function_timestamp(void *ptr, uint32_t* timestamp)
> > >> > > +anv_device_get_cache_uuid(void *uuid)
> > >> > > {
> > >> > > - Dl_info info;
> > >> > > - struct stat st;
> > >> > > - if (!dladdr(ptr, &info) || !info.dli_fname)
> > >> > > + const struct note *note = build_id_find_nhdr("libvulkan_
> intel.so");
> > >> > > + if (!note)
> > >> > > return false;
> > >> > >
> > >> > > - if (stat(info.dli_fname, &st))
> > >> > > + unsigned len = build_id_length(note);
> > >> > > + if (len < VK_UUID_SIZE)
> > >> > > return false;
> > >> > >
> > >> > > - *timestamp = st.st_mtim.tv_sec;
> > >> > > - return true;
> > >> > > -}
> > >> > > -
> > >> > > -static bool
> > >> > > -anv_device_get_cache_uuid(void *uuid)
> > >> > > -{
> > >> > > - uint32_t timestamp;
> > >> > > -
> > >> > > - memset(uuid, 0, VK_UUID_SIZE);
> > >> > > - if (!anv_get_function_timestamp(anv_device_get_cache_uuid,
> ×tamp))
> > >> > > + unsigned char *build_id = malloc(len);
> > >> > > + if (!build_id)
> > >> > > return false;
> > >> > >
> > >> > > - snprintf(uuid, VK_UUID_SIZE, "anv-%d", timestamp);
> > >> > > + build_id_read(note, build_id);
> > >> > > +
> > >> > > + memcpy(uuid, build_id, VK_UUID_SIZE);
> > >> > > + free(build_id);
> > >> >
> > >> > The Vulkan spec frowns on memory allocations when not needed. If you
> > >> > must allocate memory here, then it should be through the VkInstance
> > >> > allocation callbacks. However, it's best to avoid the allocation by
> > >> > adding a size_t parameter, à la snprintf, to build_id_read().
> > >> >
> > >> > Otherwise, the patch looks good to me.
> > >>
> > >> You're worried about the performance of anv_physical_device_init()?
> > >> This doesn't happen on lookup...just driver start up...
> > >
> > > I'm not worried about performance here. That would be foolish.
> > >
> > > I'm concerned about complying the expectations provided to advanced
> > > users by the Vulkan spec.
>
> > Hi Chad, pardon for dropping in uninvited:
> >
> > Spec mandates that only the implementation should use the allocation
> > callbacks, and not any of it's dependencies (say something in the C
> > runtime/other library), correct ?
>
> > Or does it implicitly that "Thou Shalt override the weak
> > [mc]alloc/friends symbols provided by your C runtime" ?
>
> The spec actually says neither. It is vague on this topic, but vague in
> a hyper-detailed fashion, which is typical of Khronos specs.
>
> I interpret "Chapter 10. Memory Allocation" as saying:
>
> - If the user supplies allocation callbacks, then the spec
> recommends that the implementation use them. (The spec doesn't say
> *should*, *suggest*, *must*, *shall*, or anything like that here,
> as far as I can tell. It's my interpretation that Ch10 is primarily a
> a recommendation.
>
> - If the implementation does use an allocation callback, Ch10
> defines precise constraints on how the implementation must use it.
>
> - Ch10 admits that the implementation may need to allocate internal
> memory, without using the callbacks. However, Ch10 seem to say
> that such internal allocations are soley for executable memory.
> Again, one could interpret the spec differently here.
>
> > Although the src/intel is [almost] perfect, the i965 compiler still
> > explicitly and implicitly uses [mc]alloc. Not mean to be bashful, just
> > pointing out what may not be obvious at first.
>
> Yep. The compiler doesn't use the allocation callbacks. For that matter,
> even printf calls malloc on some libc implementations.
Yeah, I think you're worrying too much. Sure, it might be a nicer API if
get_build_id did a copy into a provided array, but I wouldn't make any
malloc arguments.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
