diff mbox series

[v3,16/28] tcg: Replace region.end with region.total_size

Message ID 20210502231844.1977630-17-richard.henderson@linaro.org
State Superseded
Headers show
Series tcg: Clean up code_gen_buffer allocation | expand

Commit Message

Richard Henderson May 2, 2021, 11:18 p.m. UTC
A size is easier to work with than an end point,
particularly during initial buffer allocation.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 tcg/region.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

-- 
2.25.1

Comments

Alex Bennée June 8, 2021, 4:03 p.m. UTC | #1
Richard Henderson <richard.henderson@linaro.org> writes:

> A size is easier to work with than an end point,

> particularly during initial buffer allocation.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  tcg/region.c | 29 +++++++++++++++++------------

>  1 file changed, 17 insertions(+), 12 deletions(-)

>

> diff --git a/tcg/region.c b/tcg/region.c

> index 9a1e039824..a17f342f38 100644

> --- a/tcg/region.c

> +++ b/tcg/region.c

> @@ -48,7 +48,7 @@ struct tcg_region_state {

>      /* fields set at init time */

>      void *start;

>      void *start_aligned;

> -    void *end;

> +    size_t total_size; /* size of entire buffer */

>      size_t n;

>      size_t size; /* size of one region */

>      size_t stride; /* .size + guard size */


I'd shuffle that to the end so it scans size < stride < total_size. I
see we have special handling bellow:

> @@ -279,7 +279,7 @@ static void tcg_region_bounds(size_t curr_region, void **pstart, void **pend)

>          start = region.start;

>      }

>      if (curr_region == region.n - 1) {

> -        end = region.end;

> +        end = region.start_aligned + region.total_size;


So why isn't this end = start_aligned + (n * stride)? do we not line up
for the last region?

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Richard Henderson June 8, 2021, 4:12 p.m. UTC | #2
On 6/8/21 9:03 AM, Alex Bennée wrote:
>> @@ -279,7 +279,7 @@ static void tcg_region_bounds(size_t curr_region, void **pstart, void **pend)

>>           start = region.start;

>>       }

>>       if (curr_region == region.n - 1) {

>> -        end = region.end;

>> +        end = region.start_aligned + region.total_size;

> 

> So why isn't this end = start_aligned + (n * stride)? do we not line up

> for the last region?


Correct.  There's some commentary to that effect in tcg_region_init, but I 
guess it could stand to be copied here.


r~
Luis Fernando Fujita Pires June 9, 2021, 2:58 p.m. UTC | #3
From: Richard Henderson <richard.henderson@linaro.org>

> A size is easier to work with than an end point, particularly during initial buffer

> allocation.

> 

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  tcg/region.c | 29 +++++++++++++++++------------

>  1 file changed, 17 insertions(+), 12 deletions(-)


Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>


--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
diff mbox series

Patch

diff --git a/tcg/region.c b/tcg/region.c
index 9a1e039824..a17f342f38 100644
--- a/tcg/region.c
+++ b/tcg/region.c
@@ -48,7 +48,7 @@  struct tcg_region_state {
     /* fields set at init time */
     void *start;
     void *start_aligned;
-    void *end;
+    size_t total_size; /* size of entire buffer */
     size_t n;
     size_t size; /* size of one region */
     size_t stride; /* .size + guard size */
@@ -279,7 +279,7 @@  static void tcg_region_bounds(size_t curr_region, void **pstart, void **pend)
         start = region.start;
     }
     if (curr_region == region.n - 1) {
-        end = region.end;
+        end = region.start_aligned + region.total_size;
     }
 
     *pstart = start;
@@ -813,8 +813,8 @@  static bool alloc_code_gen_buffer(size_t size, int splitwx, Error **errp)
  */
 void tcg_region_init(size_t tb_size, int splitwx, unsigned max_cpus)
 {
-    void *buf, *aligned;
-    size_t size;
+    void *buf, *aligned, *end;
+    size_t total_size;
     size_t page_size;
     size_t region_size;
     size_t n_regions;
@@ -826,19 +826,20 @@  void tcg_region_init(size_t tb_size, int splitwx, unsigned max_cpus)
     assert(ok);
 
     buf = tcg_init_ctx.code_gen_buffer;
-    size = tcg_init_ctx.code_gen_buffer_size;
+    total_size = tcg_init_ctx.code_gen_buffer_size;
     page_size = qemu_real_host_page_size;
     n_regions = tcg_n_regions(max_cpus);
 
     /* The first region will be 'aligned - buf' bytes larger than the others */
     aligned = QEMU_ALIGN_PTR_UP(buf, page_size);
-    g_assert(aligned < tcg_init_ctx.code_gen_buffer + size);
+    g_assert(aligned < tcg_init_ctx.code_gen_buffer + total_size);
+
     /*
      * Make region_size a multiple of page_size, using aligned as the start.
      * As a result of this we might end up with a few extra pages at the end of
      * the buffer; we will assign those to the last region.
      */
-    region_size = (size - (aligned - buf)) / n_regions;
+    region_size = (total_size - (aligned - buf)) / n_regions;
     region_size = QEMU_ALIGN_DOWN(region_size, page_size);
 
     /* A region must have at least 2 pages; one code, one guard */
@@ -852,9 +853,11 @@  void tcg_region_init(size_t tb_size, int splitwx, unsigned max_cpus)
     region.start = buf;
     region.start_aligned = aligned;
     /* page-align the end, since its last page will be a guard page */
-    region.end = QEMU_ALIGN_PTR_DOWN(buf + size, page_size);
+    end = QEMU_ALIGN_PTR_DOWN(buf + total_size, page_size);
     /* account for that last guard page */
-    region.end -= page_size;
+    end -= page_size;
+    total_size = end - aligned;
+    region.total_size = total_size;
 
     /*
      * Set guard pages in the rw buffer, as that's the one into which
@@ -895,7 +898,7 @@  void tcg_region_prologue_set(TCGContext *s)
 
     /* Register the balance of the buffer with gdb. */
     tcg_register_jit(tcg_splitwx_to_rx(region.start),
-                     region.end - region.start);
+                     region.start_aligned + region.total_size - region.start);
 }
 
 /*
@@ -936,8 +939,10 @@  size_t tcg_code_capacity(void)
 
     /* no need for synchronization; these variables are set at init time */
     guard_size = region.stride - region.size;
-    capacity = region.end + guard_size - region.start;
-    capacity -= region.n * (guard_size + TCG_HIGHWATER);
+    capacity = region.total_size;
+    capacity -= (region.n - 1) * guard_size;
+    capacity -= region.n * TCG_HIGHWATER;
+
     return capacity;
 }