[igt-dev] [PATCH igt] igt/gem_exec_schedule: Trim max number of contexts used

[Chris Wilson]

Quoting Antonio Argenziano (2018-02-13 23:51:00)
> 
> 
> On 13/02/18 15:10, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2018-02-13 22:47:35)
> >>
> >>
> >> On 13/02/18 01:26, Chris Wilson wrote:
> >>> icl offers a much reduced context space, and in its simplest setup we
> >>> cannot allocate one context per priority level, so trim the number and
> >>> reuse the same context for multiple priority requests.
> >>>
> >>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>> ---
> >>>    tests/gem_exec_schedule.c | 38 +++++++++++++++++++++++---------------
> >>>    1 file changed, 23 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c
> >>> index 05f7ae38..ec02d994 100644
> >>> --- a/tests/gem_exec_schedule.c
> >>> +++ b/tests/gem_exec_schedule.c
> >>> @@ -43,6 +43,8 @@
> >>>    #define BUSY_QLEN 8
> >>>    #define MAX_ELSP_QLEN 16
> >>>    
> >>> +#define MAX_CONTEXTS 256
> >>> +
> >>>    IGT_TEST_DESCRIPTION("Check that we can control the order of execution");
> >>>    
> >>>    static void store_dword(int fd, uint32_t ctx, unsigned ring,
> >>> @@ -556,8 +558,8 @@ static void preemptive_hang(int fd, unsigned ring)
> >>>    static void deep(int fd, unsigned ring)
> >>>    {
> >>>    #define XS 8
> >>> -     const unsigned int nctx = MAX_PRIO - MIN_PRIO;
> >>> -     const unsigned size = ALIGN(4*nctx, 4096);
> >>> +     const unsigned int nreq = MAX_PRIO - MIN_PRIO;
> >>> +     const unsigned size = ALIGN(4*nreq, 4096);
> >>>        struct timespec tv = {};
> >>>        struct cork cork;
> >>>        uint32_t result, dep[XS];
> >>> @@ -565,10 +567,9 @@ static void deep(int fd, unsigned ring)
> >>>        uint32_t *ptr;
> >>>        uint32_t *ctx;
> >>>    
> >>> -     ctx = malloc(sizeof(*ctx) * nctx);
> >>> -     for (int n = 0; n < nctx; n++) {
> >>> +     ctx = malloc(sizeof(*ctx) * MAX_CONTEXTS);
> >>> +     for (int n = 0; n < MAX_CONTEXTS; n++) {
> >>>                ctx[n] = gem_context_create(fd);
> >>> -             gem_context_set_priority(fd, ctx[n], MAX_PRIO - nctx + n);
> >>>        }
> >>>    
> >>>        result = gem_create(fd, size);
> >>> @@ -592,7 +593,7 @@ static void deep(int fd, unsigned ring)
> >>>                execbuf.buffers_ptr = to_user_pointer(obj);
> >>>                execbuf.buffer_count = XS + 2;
> >>>                execbuf.flags = ring;
> >>> -             for (int n = 0; n < nctx; n++) {
> >>> +             for (int n = 0; n < MAX_CONTEXTS; n++) {
> >>>                        execbuf.rsvd1 = ctx[n];
> >>>                        gem_execbuf(fd, &execbuf);
> >>>                }
> >>> @@ -603,22 +604,29 @@ static void deep(int fd, unsigned ring)
> >>>        plug(fd, &cork);
> >>>    
> >>>        /* Create a deep dependency chain, with a few branches */
> >>> -     for (int n = 0; n < nctx && igt_seconds_elapsed(&tv) < 8; n++)
> >>> +     for (int n = 0; n < nreq && igt_seconds_elapsed(&tv) < 8; n++) {
> >>> +             uint32_t context = ctx[n % MAX_CONTEXTS];
> >>
> >> Doesn't this introduce some intra-context dependency we didn't have
> >> before? Do we care?
> > 
> > Yes, it reduces the re-ordering possibilities (it will also exercise
> > priority inversion within the context as well). We have to reduce the
> > test somehow, and this preserves the essence in that the kernel has to
> > manage the same long dependencies chains, just now with a few more
> > branches.
> 
> OK.
> 
> Acked-by: Antonio Argenziano <antonio.argenziano at intel.com>

Fwiw, we can bump MAX_CONTEXTS to 1024.
-Chris