[Intel-gfx] [PATCH v4 1/7] drm/i915: Implement a framework for batch buffer pools

[Michael H. Nguyen]

Hi Daniel, Chris

On 11/12/2014 08:38 AM, Chris Wilson wrote:
> On Wed, Nov 12, 2014 at 05:33:08PM +0100, Daniel Vetter wrote:
>> On Wed, Nov 12, 2014 at 10:46 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>>> On Wed, Nov 12, 2014 at 09:44:34AM +0100, Daniel Vetter wrote:
>>>> On Fri, Nov 07, 2014 at 02:22:01PM -0800, bradley.d.volkin at intel.com wrote:
>>>>> +           if (obj && obj->madv == __I915_MADV_PURGED) {
>>>>> +                   was_purged = true;
>>>>> +                   list_del(&obj->batch_pool_list);
>>>>> +                   drm_gem_object_unreference(&obj->base);
>>>>> +                   obj = NULL;
>>>>> +           }
>>>>
>>>> Minor issue: We should move the purged check into the loop so that purge
>>>> buffer structs get released even when they're too small/big. Otherwise
>>>> we'll have a good chance to hang onto gobloads of structs forever.
>>>
>>> I mentioned that we should do the purge of structs in our oom-notifier
>>> as well to be safe.
I understand Daniel's suggestion to move the purge check into the loop 
(will do that) but I'm not familiar w/ the oom-notifier at all and so 
don't know how to do what Chris is asking w/ out ramping up. Is it not 
critical, follow up type work or is it absolutely necessary to have 
before merge? It sounded like the first.

Thx,
Mike
>>
>> Given that we don't purge the sturcts for userspace purged objects (we
>> could just clear everything but the idr slot and mark it specially) I
>> don't think we need this here. At least not until we have this for
>> userspace bos since there's lots more of those I think.
>
> Well discarding userspace purged objects requires a special zombie
> handle, but these pure in-kernel structs we have complete control over
> and so are much simpler to clean up.
> -Chris
>