[igt-dev] [PATCH i-g-t 2/2] lib/intel_allocator: Move ioctl calls to client processes
Zbigniew Kempczyński
zbigniew.kempczynski at intel.com
Fri Jun 18 09:57:50 UTC 2021
On Thu, Jun 17, 2021 at 09:16:43AM +0200, Andrzej Turko wrote:
> When allocator is running in multiprocess mode, all queries
> are processed in a designated thread in the parent process.
> However, a child process may request opening the allocator
> for a gpu using a file descriptor absent in the parent process.
> Hence, querying available gtt size must be done in the child
> instead of the parent process.
>
> As a consequence of this change creating allocators managing
> only a given interval of available gtt is now enabled for all
> allocator implementations. Additionally, this commit sets a
> universal lower bound on alignment to 4096.
>
> Signed-off-by: Andrzej Turko <andrzej.turko at linux.intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> ---
> lib/intel_allocator.c | 51 +++++++++++++++++++++++++-----------
> lib/intel_allocator_random.c | 47 +++++++++++++++++----------------
> lib/intel_allocator_reloc.c | 35 +++++++++++--------------
> lib/intel_allocator_simple.c | 45 ++++---------------------------
> 4 files changed, 79 insertions(+), 99 deletions(-)
>
> diff --git a/lib/intel_allocator.c b/lib/intel_allocator.c
> index c060e10c3..133176ed4 100644
> --- a/lib/intel_allocator.c
> +++ b/lib/intel_allocator.c
> @@ -45,6 +45,12 @@ static inline const char *reqstr(enum reqtype request_type)
> #define alloc_debug(...) {}
> #endif
>
> +/*
> + * We limit allocator space to avoid hang when batch would be
> + * pinned in the last page.
> + */
> +#define RESERVED 4096
> +
> struct allocator {
> int fd;
> uint32_t ctx;
> @@ -58,12 +64,13 @@ struct handle_entry {
> struct allocator *al;
> };
>
> -struct intel_allocator *intel_allocator_reloc_create(int fd);
> -struct intel_allocator *intel_allocator_random_create(int fd);
> -struct intel_allocator *intel_allocator_simple_create(int fd);
> struct intel_allocator *
> -intel_allocator_simple_create_full(int fd, uint64_t start, uint64_t end,
> - enum allocator_strategy strategy);
> +intel_allocator_reloc_create(int fd, uint64_t start, uint64_t end);
> +struct intel_allocator *
> +intel_allocator_random_create(int fd, uint64_t start, uint64_t end);
> +struct intel_allocator *
> +intel_allocator_simple_create(int fd, uint64_t start, uint64_t end,
> + enum allocator_strategy strategy);
>
> /*
> * Instead of trying to find first empty handle just get new one. Assuming
> @@ -280,8 +287,6 @@ static struct intel_allocator *intel_allocator_create(int fd,
> {
> struct intel_allocator *ial = NULL;
>
> - igt_assert(can_report_gtt_size(fd));
> -
> switch (allocator_type) {
> /*
> * Few words of explanation is required here.
> @@ -297,17 +302,14 @@ static struct intel_allocator *intel_allocator_create(int fd,
> "We cannot use NONE allocator\n");
> break;
> case INTEL_ALLOCATOR_RELOC:
> - ial = intel_allocator_reloc_create(fd);
> + ial = intel_allocator_reloc_create(fd, start, end);
> break;
> case INTEL_ALLOCATOR_RANDOM:
> - ial = intel_allocator_random_create(fd);
> + ial = intel_allocator_random_create(fd, start, end);
> break;
> case INTEL_ALLOCATOR_SIMPLE:
> - if (!start && !end)
> - ial = intel_allocator_simple_create(fd);
> - else
> - ial = intel_allocator_simple_create_full(fd, start, end,
> - allocator_strategy);
> + ial = intel_allocator_simple_create(fd, start, end,
> + allocator_strategy);
> break;
> default:
> igt_assert_f(ial, "Allocator type %d not implemented\n",
> @@ -888,6 +890,18 @@ static uint64_t __intel_allocator_open_full(int fd, uint32_t ctx,
> .open.allocator_type = allocator_type,
> .open.allocator_strategy = strategy };
> struct alloc_resp resp;
> + uint64_t gtt_size;
> +
> + if (!start && !end) {
> + igt_assert_f(can_report_gtt_size(fd), "Invalid fd\n");
> + gtt_size = gem_aperture_size(fd);
> + if (!gem_uses_full_ppgtt(fd))
> + gtt_size /= 2;
> + else
> + gtt_size -= RESERVED;
> +
> + req.open.end = gtt_size;
> + }
>
> /* Get child_tid only once at open() */
> if (child_tid == -1)
> @@ -918,6 +932,9 @@ static uint64_t __intel_allocator_open_full(int fd, uint32_t ctx,
> * Returns: unique handle to the currently opened allocator.
> *
> * Notes:
> + *
> + * If start = end = 0, the allocator is opened for the whole available gtt.
> + *
> * Strategy is generally used internally by the underlying allocator:
> *
> * For SIMPLE allocator:
> @@ -926,7 +943,7 @@ static uint64_t __intel_allocator_open_full(int fd, uint32_t ctx,
> * addresses.
> *
> * For RANDOM allocator:
> - * - none of strategy is currently implemented.
> + * - no strategy is currently implemented.
> */
> uint64_t intel_allocator_open_full(int fd, uint32_t ctx,
> uint64_t start, uint64_t end,
> @@ -1067,10 +1084,12 @@ uint64_t __intel_allocator_alloc(uint64_t allocator_handle, uint32_t handle,
> .allocator_handle = allocator_handle,
> .alloc.handle = handle,
> .alloc.size = size,
> - .alloc.alignment = alignment,
> .alloc.strategy = strategy };
> struct alloc_resp resp;
>
> + igt_assert((alignment & (alignment-1)) == 0);
> + req.alloc.alignment = max(alignment, 1 << 12);
> +
This limits our allocator use to 4K allocation granularity but I got no
problem with this now. If someone would need do smaller allocations changing
tests to specify alignment to 4K would be necessary.
> igt_assert(handle_request(&req, &resp) == 0);
> igt_assert(resp.response_type == RESP_ALLOC);
>
> diff --git a/lib/intel_allocator_random.c b/lib/intel_allocator_random.c
> index 3d9a78f17..9e3142617 100644
> --- a/lib/intel_allocator_random.c
> +++ b/lib/intel_allocator_random.c
> @@ -10,12 +10,11 @@
> #include "igt_rand.h"
> #include "intel_allocator.h"
>
> -struct intel_allocator *intel_allocator_random_create(int fd);
> +struct intel_allocator *
> +intel_allocator_random_create(int fd, uint64_t start, uint64_t end);
>
> struct intel_allocator_random {
> - uint64_t bias;
> uint32_t prng;
> - uint64_t gtt_size;
> uint64_t start;
> uint64_t end;
>
> @@ -23,12 +22,9 @@ struct intel_allocator_random {
> uint64_t allocated_objects;
> };
>
> -static uint64_t get_bias(int fd)
> -{
> - (void) fd;
> -
> - return 256 << 10;
> -}
> +/* Keep the low 256k clear, for negative deltas */
> +#define BIAS (256 << 10)
> +#define RETRIES 8
>
> static void intel_allocator_random_get_address_range(struct intel_allocator *ial,
> uint64_t *startp,
> @@ -50,6 +46,7 @@ static uint64_t intel_allocator_random_alloc(struct intel_allocator *ial,
> {
> struct intel_allocator_random *ialr = ial->priv;
> uint64_t offset;
> + int cnt = RETRIES;
>
> (void) handle;
> (void) strategy;
> @@ -57,10 +54,17 @@ static uint64_t intel_allocator_random_alloc(struct intel_allocator *ial,
> /* randomize the address, we try to avoid relocations */
> do {
> offset = hars_petruska_f54_1_random64(&ialr->prng);
> - offset += ialr->bias; /* Keep the low 256k clear, for negative deltas */
> - offset &= ialr->gtt_size - 1;
> - offset &= ~(alignment - 1);
> - } while (offset + size > ialr->end);
> + /* maximize the chances of fitting in the last iteration */
> + if (cnt == 1)
> + offset = 0;
> +
> + offset %= ialr->end - ialr->start;
> + offset += ialr->start;
> + offset = ALIGN(offset, alignment);
> + } while (offset + size > ialr->end && --cnt);
> +
> + if (!cnt)
> + return ALLOC_INVALID_ADDRESS;
Ok, deadloop fix.
>
> ialr->allocated_objects++;
>
> @@ -150,8 +154,8 @@ static bool intel_allocator_random_is_empty(struct intel_allocator *ial)
> return !ialr->allocated_objects;
> }
>
> -#define RESERVED 4096
> -struct intel_allocator *intel_allocator_random_create(int fd)
> +struct intel_allocator *
> +intel_allocator_random_create(int fd, uint64_t start, uint64_t end)
> {
> struct intel_allocator *ial;
> struct intel_allocator_random *ialr;
> @@ -175,14 +179,11 @@ struct intel_allocator *intel_allocator_random_create(int fd)
> ialr = ial->priv = calloc(1, sizeof(*ialr));
> igt_assert(ial->priv);
> ialr->prng = (uint32_t) to_user_pointer(ial);
> - ialr->gtt_size = gem_aperture_size(fd);
> - igt_debug("Gtt size: %" PRId64 "\n", ialr->gtt_size);
> - if (!gem_uses_full_ppgtt(fd))
> - ialr->gtt_size /= 2;
> -
> - ialr->bias = get_bias(fd);
> - ialr->start = ialr->bias;
> - ialr->end = ialr->gtt_size - RESERVED;
> +
> + start = max(start, BIAS);
> + igt_assert(start < end);
> + ialr->start = start;
> + ialr->end = end;
>
> ialr->allocated_objects = 0;
>
> diff --git a/lib/intel_allocator_reloc.c b/lib/intel_allocator_reloc.c
> index e8af787b0..1790e6c79 100644
> --- a/lib/intel_allocator_reloc.c
> +++ b/lib/intel_allocator_reloc.c
> @@ -10,12 +10,11 @@
> #include "igt_rand.h"
> #include "intel_allocator.h"
>
> -struct intel_allocator *intel_allocator_reloc_create(int fd);
> +struct intel_allocator *
> +intel_allocator_reloc_create(int fd, uint64_t start, uint64_t end);
>
> struct intel_allocator_reloc {
> - uint64_t bias;
> uint32_t prng;
> - uint64_t gtt_size;
> uint64_t start;
> uint64_t end;
> uint64_t offset;
> @@ -24,12 +23,8 @@ struct intel_allocator_reloc {
> uint64_t allocated_objects;
> };
>
> -static uint64_t get_bias(int fd)
> -{
> - (void) fd;
> -
> - return 256 << 10;
> -}
> +/* Keep the low 256k clear, for negative deltas */
> +#define BIAS (256 << 10)
>
> static void intel_allocator_reloc_get_address_range(struct intel_allocator *ial,
> uint64_t *startp,
> @@ -55,13 +50,16 @@ static uint64_t intel_allocator_reloc_alloc(struct intel_allocator *ial,
> (void) handle;
> (void) strategy;
>
> - alignment = max(alignment, 4096);
> aligned_offset = ALIGN(ialr->offset, alignment);
>
> /* Check we won't exceed end */
> if (aligned_offset + size > ialr->end)
> aligned_offset = ALIGN(ialr->start, alignment);
>
> + /* Check that the object fits in the address range */
> + if (aligned_offset + size > ialr->end)
> + return ALLOC_INVALID_ADDRESS;
> +
Ok, corner case fix.
> offset = aligned_offset;
> ialr->offset = offset + size;
> ialr->allocated_objects++;
> @@ -152,8 +150,8 @@ static bool intel_allocator_reloc_is_empty(struct intel_allocator *ial)
> return !ialr->allocated_objects;
> }
>
> -#define RESERVED 4096
> -struct intel_allocator *intel_allocator_reloc_create(int fd)
> +struct intel_allocator *
> +intel_allocator_reloc_create(int fd, uint64_t start, uint64_t end)
> {
> struct intel_allocator *ial;
> struct intel_allocator_reloc *ialr;
> @@ -177,14 +175,11 @@ struct intel_allocator *intel_allocator_reloc_create(int fd)
> ialr = ial->priv = calloc(1, sizeof(*ialr));
> igt_assert(ial->priv);
> ialr->prng = (uint32_t) to_user_pointer(ial);
> - ialr->gtt_size = gem_aperture_size(fd);
> - igt_debug("Gtt size: %" PRId64 "\n", ialr->gtt_size);
> - if (!gem_uses_full_ppgtt(fd))
> - ialr->gtt_size /= 2;
> -
> - ialr->bias = ialr->offset = get_bias(fd);
> - ialr->start = ialr->bias;
> - ialr->end = ialr->gtt_size - RESERVED;
> +
> + start = max(start, BIAS);
> + igt_assert(start < end);
> + ialr->offset = ialr->start = start;
> + ialr->end = end;
>
> ialr->allocated_objects = 0;
>
> diff --git a/lib/intel_allocator_simple.c b/lib/intel_allocator_simple.c
> index 963d8d257..0e6763964 100644
> --- a/lib/intel_allocator_simple.c
> +++ b/lib/intel_allocator_simple.c
> @@ -11,17 +11,11 @@
> #include "intel_bufops.h"
> #include "igt_map.h"
>
> -/*
> - * We limit allocator space to avoid hang when batch would be
> - * pinned in the last page.
> - */
> -#define RESERVED 4096
>
> /* Avoid compilation warning */
> -struct intel_allocator *intel_allocator_simple_create(int fd);
> struct intel_allocator *
> -intel_allocator_simple_create_full(int fd, uint64_t start, uint64_t end,
> - enum allocator_strategy strategy);
> +intel_allocator_simple_create(int fd, uint64_t start, uint64_t end,
> + enum allocator_strategy strategy);
>
> struct simple_vma_heap {
> struct igt_list_head holes;
> @@ -425,7 +419,6 @@ static uint64_t intel_allocator_simple_alloc(struct intel_allocator *ial,
> ials = (struct intel_allocator_simple *) ial->priv;
> igt_assert(ials);
> igt_assert(handle);
> - alignment = alignment > 0 ? alignment : 1;
>
> rec = igt_map_search(ials->objects, &handle);
> if (rec) {
> @@ -734,9 +727,9 @@ static void intel_allocator_simple_print(struct intel_allocator *ial, bool full)
> ials->allocated_objects, ials->reserved_areas);
> }
>
> -static struct intel_allocator *
> -__intel_allocator_simple_create(int fd, uint64_t start, uint64_t end,
> - enum allocator_strategy strategy)
> +struct intel_allocator *
> +intel_allocator_simple_create(int fd, uint64_t start, uint64_t end,
> + enum allocator_strategy strategy)
> {
> struct intel_allocator *ial;
> struct intel_allocator_simple *ials;
> @@ -777,31 +770,3 @@ __intel_allocator_simple_create(int fd, uint64_t start, uint64_t end,
>
> return ial;
> }
> -
> -struct intel_allocator *
> -intel_allocator_simple_create(int fd)
> -{
> - uint64_t gtt_size = gem_aperture_size(fd);
> -
> - if (!gem_uses_full_ppgtt(fd))
> - gtt_size /= 2;
> - else
> - gtt_size -= RESERVED;
> -
> - return __intel_allocator_simple_create(fd, 0, gtt_size,
> - ALLOC_STRATEGY_HIGH_TO_LOW);
> -}
> -
> -struct intel_allocator *
> -intel_allocator_simple_create_full(int fd, uint64_t start, uint64_t end,
> - enum allocator_strategy strategy)
> -{
> - uint64_t gtt_size = gem_aperture_size(fd);
> -
> - igt_assert(end <= gtt_size);
> - if (!gem_uses_full_ppgtt(fd))
> - gtt_size /= 2;
> - igt_assert(end - start <= gtt_size);
> -
> - return __intel_allocator_simple_create(fd, start, end, strategy);
> -}
> --
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
--
Zbigniew
> 2.25.1
>
More information about the igt-dev
mailing list