[Intel-gfx] [PATCH 12/41] drm/i915: Introduce an internal allocator for disposable private objects

[Chris Wilson]

On Fri, Oct 21, 2016 at 08:21:08AM +0100, Tvrtko Ursulin wrote:
> 
> On 20/10/2016 21:36, Chris Wilson wrote:
> >On Thu, Oct 20, 2016 at 05:22:23PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 20/10/2016 16:03, Chris Wilson wrote:
> >>>Quite a few of our objects used for internal hardware programming do not
> >>>benefit from being swappable or from being zero initialised. As such
> >>>they do not benefit from using a shmemfs backing storage and since they
> >>>are internal and never directly exposed to the user, we do not need to
> >>>worry about providing a filp. For these we can use an
> >>>drm_i915_gem_object wrapper around a sg_table of plain struct page. They
> >>>are not swap backed and not automatically pinned. If they are reaped
> >>>by the shrinker, the pages are released and the contents discarded. For
> >>>the internal use case, this is fine as for example, ringbuffers are
> >>>pinned from being written by a request to be read by the hardware. Once
> >>>they are idle, they can be discarded entirely. As such they are a good
> >>>match for execlist ringbuffers and a small variety of other internal
> >>>objects.
> >>>
> >>>In the first iteration, this is limited to the scratch batch buffers we
> >>>use (for command parsing and state initialisation).
> >>
> >>And the status page.
> >
> >Yeah, I was just thinking of the runtime allocated blocks where the
> >change can be measured.
> >
> >>>+	max_order = MAX_ORDER;
> >>>+#ifdef CONFIG_SWIOTLB
> >>>+	if (swiotlb_nr_tbl())
> >>>+		max_order = min(max_order, ilog2(IO_TLB_SEGSIZE));
> >>>+#endif
> >>
> >>I couldn't figure out what IO_TLB_SEGSIZE actually is in some
> >>minutes of cross-referencing. Did not seem to be in units of bytes
> >>according to swiotlb.h.
> >
> >Pages.
> >
> >>In either case my question is - why use different parameters than
> >>swiotlb_max_size you recently added to i915_gem.c?
> >
> >I was trying to exploit the compile time constants, and I did not care
> >to grow the search for even higher orders.
> 
> Sorry pages, but it is not AFAICS, but it is units of IO_TLB_SHIFT
> size which is 2k. Happens to be fine since it is smaller (5) than
> MAX_ORDER (11), however to me it still looks like a unrelated
> number.
> 
> ilog2(IO_TLB_SEGSIZE) + 1 would be the same units. It would still
> look arbitrary but I suppose it would be passable.

Oh, no! I do remember that. I guess that shows the importance of
comments since I lost that fact during the chase to find a way of
writing fls() as a compiletime constant.

Let's

/* convert swiotlb segment size into sensible units! */ 
#define IO_TLB_SEGPAGES (IO_TLB_SEGSIZE << IO_TLB_SHIFT >> PAGE_SHIFT)

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre