diff mbox series

[v3,04/28] tcg: Remove error return from tcg_region_initial_alloc__locked

Message ID 20210502231844.1977630-5-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 
All callers immediately assert on error, so move the assert
into the function itself.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

---
 tcg/tcg.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

-- 
2.25.1

Comments

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

> All callers immediately assert on error, so move the assert

> into the function itself.

>

> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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


The change itself seems reasonable although I do find the return values
of the underlying tcg_region_alloc__locked a little confusing. What we
are saying is the initial allocation should never fail but subsequent
allocations aren't actually fails but creation of new regions?

Anyway:

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


-- 
Alex Bennée
Richard Henderson June 8, 2021, 3:44 p.m. UTC | #2 
On 6/8/21 4:04 AM, Alex Bennée wrote:
> 

> Richard Henderson <richard.henderson@linaro.org> writes:

> 

>> All callers immediately assert on error, so move the assert

>> into the function itself.

>>

>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> 

> The change itself seems reasonable although I do find the return values

> of the underlying tcg_region_alloc__locked a little confusing. What we

> are saying is the initial allocation should never fail but subsequent

> allocations aren't actually fails but creation of new regions?


Sort of.  If we get a failure during initial allocation, it's because we didn't 
actually create enough regions (ncpu > nregions).

During the normal course of events, a failure indicates that we've run out of 
clean regions and need to flush the entire buffer.


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

> All callers immediately assert on error, so move the assert into the function

> itself.

> 

> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---

>  tcg/tcg.c | 19 ++++++-------------

>  1 file changed, 6 insertions(+), 13 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/tcg.c b/tcg/tcg.c
index 795a71ff25..8b57e93ca2 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -723,9 +723,10 @@  static bool tcg_region_alloc(TCGContext *s)
  * Perform a context's first region allocation.
  * This function does _not_ increment region.agg_size_full.
  */
-static inline bool tcg_region_initial_alloc__locked(TCGContext *s)
+static void tcg_region_initial_alloc__locked(TCGContext *s)
 {
-    return tcg_region_alloc__locked(s);
+    bool err = tcg_region_alloc__locked(s);
+    g_assert(!err);
 }
 
 /* Call from a safe-work context */
@@ -740,9 +741,7 @@  void tcg_region_reset_all(void)
 
     for (i = 0; i < n_ctxs; i++) {
         TCGContext *s = qatomic_read(&tcg_ctxs[i]);
-        bool err = tcg_region_initial_alloc__locked(s);
-
-        g_assert(!err);
+        tcg_region_initial_alloc__locked(s);
     }
     qemu_mutex_unlock(&region.lock);
 
@@ -879,11 +878,7 @@  void tcg_region_init(void)
 
     /* In user-mode we support only one ctx, so do the initial allocation now */
 #ifdef CONFIG_USER_ONLY
-    {
-        bool err = tcg_region_initial_alloc__locked(tcg_ctx);
-
-        g_assert(!err);
-    }
+    tcg_region_initial_alloc__locked(tcg_ctx);
 #endif
 }
 
@@ -945,7 +940,6 @@  void tcg_register_thread(void)
     MachineState *ms = MACHINE(qdev_get_machine());
     TCGContext *s = g_malloc(sizeof(*s));
     unsigned int i, n;
-    bool err;
 
     *s = tcg_init_ctx;
 
@@ -969,8 +963,7 @@  void tcg_register_thread(void)
 
     tcg_ctx = s;
     qemu_mutex_lock(&region.lock);
-    err = tcg_region_initial_alloc__locked(tcg_ctx);
-    g_assert(!err);
+    tcg_region_initial_alloc__locked(s);
     qemu_mutex_unlock(&region.lock);
 }
 #endif /* !CONFIG_USER_ONLY */