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" ?
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.
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
