On 14/06/17 05:42 PM, Nicolai Hähnle wrote:
> On 14.06.2017 10:05, Michel Dänzer wrote:
>> On 14/06/17 04:42 PM, Nicolai Hähnle wrote:
>>> On 14.06.2017 05:39, Michel Dänzer wrote:
>>>> From: Michel Dänzer <michel.daen...@amd.com>
>>>>
>>>> diff --git a/src/gallium/auxiliary/util/u_inlines.h
>>>> b/src/gallium/auxiliary/util/u_inlines.h
>>>> index 6a3d5043cf..08326da03d 100644
>>>> --- a/src/gallium/auxiliary/util/u_inlines.h
>>>> +++ b/src/gallium/auxiliary/util/u_inlines.h
>>>> @@ -137,8 +137,19 @@ pipe_resource_reference(struct pipe_resource
>>>> **ptr, struct pipe_resource *tex)
>>>> if (pipe_reference_described(&(*ptr)->reference,
>>>> &tex->reference,
>>>>
>>>> (debug_reference_descriptor)debug_describe_resource)) {
>>>> - pipe_resource_reference(&old_tex->next, NULL);
>>>> + struct pipe_resource *next = old_tex->next;
>>>> +
>>>> old_tex->screen->resource_destroy(old_tex->screen, old_tex);
>>>> +
>>>> + /* Avoid recursion, which would prevent inlining this
>>>> function */
>>>> + while (next) {
>>>> + old_tex = next;
>>>> + next = old_tex->next;
>>>> + old_tex->next = NULL;
>>>
>>> Is this really necessary? I don't think so, but if it is, it's probably
>>> necessary above as well.
>>
>> The old_tex->next = NULL assignment is necessary here to complement the
>> pipe_reference_described call below it. It's not necessary above,
>> because the ->next chain only needs to be unreferenced when the "master"
>> resource is destroyed.
>>
>> The behaviour is basically the same as with the recursive
>> pipe_resource_reference(&old_tex->next, NULL) call before: the next
>> pointers are left untouched unless the "master" resource is destroyed,
>> in which case the next pointers are set to NULL and the reference counts
>> of the corresponding sub-resources decremented (and the sub-resource
>> destroyed if it goes to 0). The only slight change is that the next
>> member of the "master" resource isn't set to NULL, but that doesn't
>> matter because it's getting destroyed anyway.
>
> Well, if (*ptr)->next is _not_ destroyed, then the new code will set
> (*ptr)->next->next to NULL anyway, while the old code would not have
> modified it.
You're right. I'm not sure it makes a difference in practice, but let's
be conservative, as that allows making the patch (and the generated
code) even smaller:
> Come to think of it, the new code should really break out of the loop
> once pipe_reference_described returns false. Maybe:
>
> do {
> struct pipe_resource *next = old_tex->next;
> old_tex->screen->resource_destroy(old_tex->screen, old_tex);
> old_tex = next;
> } while (old_tex && pipe_reference_described(&old_tex->reference, NULL,
> ...));
>
> How does that sound?
Great! I took your suggestion almost verbatim (no need to test old_tex
separately before calling pipe_reference_described) for v4, thanks.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
