All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
To: Andrzej Turko <andrzej.turko@linux.intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t 3/3] lib/intel_allocator: Add support for regions of allocation
Date: Fri, 11 Jun 2021 10:57:35 +0200	[thread overview]
Message-ID: <20210611085735.GE3298@zkempczy-mobl2> (raw)
In-Reply-To: <20210609131522.1794-4-andrzej.turko@linux.intel.com>

On Wed, Jun 09, 2021 at 03:15:22PM +0200, Andrzej Turko wrote:
> Enable creating allocators managing only a given interval
> of available gtt for all implementations, not just the
> simple allocator. Additinally, set a universal lower
> bound on alignment to 4096.
> 
> Signed-off-by: Andrzej Turko <andrzej.turko@linux.intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>  lib/intel_allocator.c        | 14 +++++++++-----
>  lib/intel_allocator_random.c | 30 +++++++++++++++++++-----------
>  lib/intel_allocator_reloc.c  | 18 ++++++++++++------
>  lib/intel_allocator_simple.c |  1 -
>  4 files changed, 40 insertions(+), 23 deletions(-)
> 
> diff --git a/lib/intel_allocator.c b/lib/intel_allocator.c
> index a78a661b1..82875224d 100644
> --- a/lib/intel_allocator.c
> +++ b/lib/intel_allocator.c
> @@ -64,8 +64,10 @@ struct handle_entry {
>  	struct allocator *al;
>  };
>  
> -struct intel_allocator *intel_allocator_reloc_create(int fd, uint64_t end);
> -struct intel_allocator *intel_allocator_random_create(int fd, uint64_t end);
> +struct intel_allocator *
> +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);
> @@ -291,10 +293,10 @@ 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, end);
> +		ial = intel_allocator_reloc_create(fd, start, end);
>  		break;
>  	case INTEL_ALLOCATOR_RANDOM:
> -		ial = intel_allocator_random_create(fd, end);
> +		ial = intel_allocator_random_create(fd, start, end);
>  		break;
>  	case INTEL_ALLOCATOR_SIMPLE:
>  		ial = intel_allocator_simple_create(fd, start, end,
> @@ -1078,10 +1080,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);
> +
>  	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 85ac2cf4e..1d2969618 100644
> --- a/lib/intel_allocator_random.c
> +++ b/lib/intel_allocator_random.c
> @@ -10,12 +10,12 @@
>  #include "igt_rand.h"
>  #include "intel_allocator.h"
>  
> -struct intel_allocator *intel_allocator_random_create(int fd, uint64_t end);
> +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 address_mask;
> +	uint64_t size_mask;
>  	uint64_t start;
>  	uint64_t end;
>  
> @@ -23,6 +23,7 @@ struct intel_allocator_random {
>  	uint64_t allocated_objects;
>  };
>  
> +/* 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,
> @@ -45,16 +46,22 @@ static uint64_t intel_allocator_random_alloc(struct intel_allocator *ial,
>  {
>  	struct intel_allocator_random *ialr = ial->priv;
>  	uint64_t offset;
> +	int cnt = 0;

int cnt = RETRIES;

>  
>  	(void) handle;
>  	(void) strategy;
>  
>  	/* randomize the address, we try to avoid relocations */
>  	do {
> +		/* the object won't fit in the address range */
> +		if (cnt > 3) return ALLOC_INVALID_ADDRESS;
> +
>  		offset = hars_petruska_f54_1_random64(&ialr->prng);
> -		offset += ialr->bias; /* Keep the low 256k clear, for negative deltas */
> -		offset &= ialr->address_mask;
> -		offset &= ~(alignment - 1);
> +		if (cnt++ == 3) offset = 0;
> +
> +		offset &= ialr->size_mask;
> +		offset += ialr->start;
> +		offset = ALIGN(offset, alignment);
>  	} while (offset + size > ialr->end);

Maybe:

/* We won't fit at all */
if (size > ialr->end - ialr->start)
	return ALLOC_INVALID_ADDRESS;

do {
	offset = hars_petruska_f54_1_random64(&ialr->prng);
	offset &= ialr->size_mask;
	offset += ialr->start;
	offset = ALIGN(offset, alignment);
} while (offset + size > ialr->end && cnt--);

if (!cnt)
	return ALLOC_INVALID_ADDRESS;

will be more readable?  

>  
>  	ialr->allocated_objects++;
> @@ -145,7 +152,8 @@ static bool intel_allocator_random_is_empty(struct intel_allocator *ial)
>  	return !ialr->allocated_objects;
>  }
>  
> -struct intel_allocator *intel_allocator_random_create(int fd, uint64_t end)
> +struct intel_allocator *
> +intel_allocator_random_create(int fd, uint64_t start, uint64_t end)
>  {
>  	struct intel_allocator *ial;
>  	struct intel_allocator_random *ialr;
> @@ -170,10 +178,10 @@ struct intel_allocator *intel_allocator_random_create(int fd, uint64_t end)
>  	igt_assert(ial->priv);
>  	ialr->prng = (uint32_t) to_user_pointer(ial);
>  
> -	igt_assert(BIAS < end);
> -	ialr->address_mask = (1ULL << (igt_fls(end) - 1)) - 1;
> -	ialr->bias = BIAS;
> -	ialr->start = ialr->bias;
> +	start = max(start, BIAS);
> +	igt_assert(start < end);
> +	ialr->size_mask = (1ULL << (igt_fls(end - start) - 1)) - 1;
> +	ialr->start = start;
>  	ialr->end = end;

No, mask will be invalid in this case.

--
Zbigniew

>  
>  	ialr->allocated_objects = 0;
> diff --git a/lib/intel_allocator_reloc.c b/lib/intel_allocator_reloc.c
> index 703d054c6..24b2aeebc 100644
> --- a/lib/intel_allocator_reloc.c
> +++ b/lib/intel_allocator_reloc.c
> @@ -10,10 +10,10 @@
>  #include "igt_rand.h"
>  #include "intel_allocator.h"
>  
> -struct intel_allocator *intel_allocator_reloc_create(int fd, uint64_t end);
> +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 start;
>  	uint64_t end;
> @@ -23,6 +23,7 @@ struct intel_allocator_reloc {
>  	uint64_t allocated_objects;
>  };
>  
> +/* Keep the low 256k clear, for negative deltas */
>  #define BIAS 256 << 10
>  
>  static void intel_allocator_reloc_get_address_range(struct intel_allocator *ial,
> @@ -49,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;
> +
>  	offset = aligned_offset;
>  	ialr->offset = offset + size;
>  	ialr->allocated_objects++;
> @@ -146,7 +150,8 @@ static bool intel_allocator_reloc_is_empty(struct intel_allocator *ial)
>  	return !ialr->allocated_objects;
>  }
>  
> -struct intel_allocator *intel_allocator_reloc_create(int fd, uint64_t end)
> +struct intel_allocator *
> +intel_allocator_reloc_create(int fd, uint64_t start, uint64_t end)
>  {
>  	struct intel_allocator *ial;
>  	struct intel_allocator_reloc *ialr;
> @@ -171,8 +176,9 @@ struct intel_allocator *intel_allocator_reloc_create(int fd, uint64_t end)
>  	igt_assert(ial->priv);
>  	ialr->prng = (uint32_t) to_user_pointer(ial);
>  
> -	ialr->bias = ialr->offset = BIAS;
> -	ialr->start = ialr->bias;
> +	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 6022e832b..0e6763964 100644
> --- a/lib/intel_allocator_simple.c
> +++ b/lib/intel_allocator_simple.c
> @@ -419,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) {
> -- 
> 2.25.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2021-06-11  8:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 13:15 [igt-dev] [PATCH v4 i-g-t 0/3] Fix the multiprocess mode of intel allocator Andrzej Turko
2021-06-09 13:15 ` [igt-dev] [PATCH i-g-t 1/3] tests/i915/api_intel_allocator: Exercise allocator in multiprocess mode Andrzej Turko
2021-06-11  8:15   ` Zbigniew Kempczyński
2021-06-09 13:15 ` [igt-dev] [PATCH i-g-t 2/3] lib/intel_allocator: Move the ioctl calls to client processes Andrzej Turko
2021-06-11  8:24   ` Zbigniew Kempczyński
2021-06-09 13:15 ` [igt-dev] [PATCH i-g-t 3/3] lib/intel_allocator: Add support for regions of allocation Andrzej Turko
2021-06-11  8:57   ` Zbigniew Kempczyński [this message]
2021-06-09 14:22 ` [igt-dev] ✓ Fi.CI.BAT: success for Fix the multiprocess mode of intel allocator Patchwork
2021-06-09 16:10 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2021-05-27 19:27 [igt-dev] [PATCH i-g-t v3 0/3] " Andrzej Turko
2021-05-27 19:27 ` [igt-dev] [PATCH i-g-t 3/3] lib/intel_allocator: Add support for regions of allocation Andrzej Turko

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=20210611085735.GE3298@zkempczy-mobl2 \
    --to=zbigniew.kempczynski@intel.com \
    --cc=andrzej.turko@linux.intel.com \
    --cc=igt-dev@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.