All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael H. Nguyen" <michael.h.nguyen@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v4 1/7] drm/i915: Implement a framework for batch buffer pools
Date: Fri, 21 Nov 2014 17:28:11 -0800	[thread overview]
Message-ID: <546FE6AB.9030706@intel.com> (raw)
In-Reply-To: <20141112163800.GQ8220@nuc-i3427.alporthouse.com>

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@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@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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2014-11-22  1:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-07 22:22 [PATCH v4 0/7] Command parser batch buffer copy bradley.d.volkin
2014-11-07 22:22 ` [PATCH v4 1/7] drm/i915: Implement a framework for batch buffer pools bradley.d.volkin
2014-11-12  8:44   ` Daniel Vetter
2014-11-12  9:46     ` Chris Wilson
2014-11-12 16:33       ` Daniel Vetter
2014-11-12 16:38         ` Chris Wilson
2014-11-22  1:28           ` Michael H. Nguyen [this message]
2014-11-24  9:18             ` Daniel Vetter
2014-11-12  9:42   ` Chris Wilson
2014-11-07 22:22 ` [PATCH v4 2/7] drm/i915: Use batch pools with the command parser bradley.d.volkin
2014-11-12  9:49   ` Chris Wilson
2014-11-07 22:22 ` [PATCH v4 3/7] drm/i915: Add a batch pool debugfs file bradley.d.volkin
2014-11-07 22:22 ` [PATCH v4 4/7] drm/i915: Add batch pool details to i915_gem_objects debugfs bradley.d.volkin
2014-11-07 22:22 ` [PATCH v4 5/7] drm/i915: Use batch length instead of object size in command parser bradley.d.volkin
2014-11-07 22:22 ` [PATCH v4 6/7] drm/i915: Mark shadow batch buffers as purgeable bradley.d.volkin
2014-11-12  8:46   ` Daniel Vetter
2014-11-07 22:22 ` [PATCH v4 7/7] drm/i915: Tidy up execbuffer command parsing code bradley.d.volkin
2014-11-07 22:25   ` [PATCH v4 7/7] drm/i915: Tidy up execbuffer command shuang.he
2014-11-12  9:37   ` [PATCH v4 7/7] drm/i915: Tidy up execbuffer command parsing code Chris Wilson
2014-11-22  1:17     ` Michael H. Nguyen
2014-11-24  9:20       ` Daniel Vetter
2014-11-12  8:51 ` [PATCH v4 0/7] Command parser batch buffer copy Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=546FE6AB.9030706@intel.com \
    --to=michael.h.nguyen@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.