[Intel-gfx] [PATCH 07/25] drm/i915: Cache the error string

[Chris Wilson]

Quoting Joonas Lahtinen (2018-11-23 12:52:28)
> Quoting Chris Wilson (2018-11-02 18:12:14)
> > Currently, we convert the error state into a string every time we read
> > from sysfs (and sysfs reads in page size (4KiB) chunks). We do try to
> > window the string and only capture the portion that is being read, but
> > that means that we must always convert up to the window to find the
> > start. For a very large error state bordering on EXEC_OBJECT_CAPTURE
> > abuse, this is noticeable as it degrades to O(N^2)!
> > 
> > As we do not have a convenient hook for sysfs open(), and we would like
> > to keep the lazy conversion into a string, do the conversion of the
> > whole string on the first read and keep the string until the error state
> > is freed.
> > 
> > v2: Don't double advance simple_read_from_buffer
> > v3: Due to extreme pain of lack of vrealloc, use a scatterlist
> > v4: Keep the forward iterator loosely cached
> > 
> > Reported-by: Jason Ekstrand <jason at jlekstrand.net>
> > Testcase: igt/gem_exec_capture/many*
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Cc: Jason Ekstrand <jason at jlekstrand.net>

> > -static bool __i915_error_ok(struct drm_i915_error_state_buf *e)
> > +static void __sg_set_buf(struct scatterlist *sg,
> > +                        void *addr, unsigned int len, loff_t it)
> >  {
> > -
> > -       if (!e->err && WARN(e->bytes > (e->size - 1), "overflow")) {
> > -               e->err = -ENOSPC;
> > -               return false;
> > -       }
> > -
> > -       if (e->bytes == e->size - 1 || e->err)
> > -               return false;
> > -
> > -       return true;
> > +       sg->page_link = (unsigned long)virt_to_page(addr);
> > +       sg->offset = offset_in_page(addr);
> > +       sg->length = len;
> > +       sg->dma_address = it;
> >  }
> 
> The least we can do is to extract the sg functions as a follow-up patch
> and try to haggle them to scatterlist.h.

[snip]

> This is (with the assumption that sg innards are extracted as
> a follow-up);

I wouldn't necessarily say sg, perhaps a chunked_array_t. The other
genarray tend to use a radixtree which seems overkill for our densely
allocated linear blocks. Tvrtko made a good suggestion to try and back
it with shmemfs to allow swappable kernel allocations (and gives us the
radixtree!), which is the direction most tempting to take. I wanted to
postpone that for later work having something that solved the problem to
hand (with a chunked allocation I'm familiar with).
-Chris