Re: [PATCH v3 06/13] rv: Do not rely on clean monitor when initialising HA

[Nam Cao]

Gabriele Monaco <gmon...@redhat.com> writes:
> +static bool ha_mon_initializing;

The global variable makes me a bit uncomfortable (a quick google will
tell why this is not the best pattern).

I am sure there are better ways to differentiate when we are
initializing vs destroying. How about the incomplete sketch below? I
doubt it even builds, just give an idea.

Nam

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 39765ff6f098..0549d3a35ee0 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -159,9 +159,14 @@ static struct da_monitor *da_get_monitor(void)
 /*
  * da_monitor_reset_all - reset the single monitor
  */
-static void da_monitor_reset_all(void)
+static void da_monitor_reset_all(void (*reset)(struct da_monitor *da_mon))
 {
-       da_monitor_reset(da_get_monitor());
+       fn(da_get_monitor());
+}
+
+static inline void _da_monitor_init(struct da_monitor *da_mon)
+{
+       memset(da_monitor, 0, sizeof(*da_mon));
 }
 
 /*
@@ -169,7 +174,7 @@ static void da_monitor_reset_all(void)
  */
 static inline int da_monitor_init(void)
 {
-       da_monitor_reset_all();
+       da_monitor_reset_all(_da_monitor_init);
        return 0;
 }
 
@@ -178,7 +183,7 @@ static inline int da_monitor_init(void)
  */
 static inline void da_monitor_destroy(void)
 {
-       da_monitor_reset_all();
+       da_monitor_reset_all(da_monitor_reset);
 }
 
 #elif RV_MON_TYPE == RV_MON_PER_CPU
@@ -202,14 +207,14 @@ static struct da_monitor *da_get_monitor(void)
 /*
  * da_monitor_reset_all - reset all CPUs' monitor
  */
-static void da_monitor_reset_all(void)
+static void da_monitor_reset_all(void (*reset)(struct da_monitor *da_mon))
 {
        struct da_monitor *da_mon;
        int cpu;
 
        for_each_cpu(cpu, cpu_online_mask) {
                da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
-               da_monitor_reset(da_mon);
+               reset(da_mon);
        }
 }
 
@@ -267,16 +272,16 @@ static inline da_id_type da_get_id(struct da_monitor 
*da_mon)
        return da_get_target(da_mon)->pid;
 }
 
-static void da_monitor_reset_all(void)
+static void da_monitor_reset_all(void (*reset)(struct da_monitor *da_mon))
 {
        struct task_struct *g, *p;
        int cpu;
 
        read_lock(&tasklist_lock);
        for_each_process_thread(g, p)
-               da_monitor_reset(da_get_monitor(p));
+               reset(da_get_monitor(p));
        for_each_present_cpu(cpu)
-               da_monitor_reset(da_get_monitor(idle_task(cpu)));
+               reset(da_get_monitor(idle_task(cpu)));
        read_unlock(&tasklist_lock);
 }
 
@@ -483,14 +488,14 @@ static inline void da_destroy_storage(da_id_type id)
        kfree_rcu(mon_storage, rcu);
 }
 
-static void da_monitor_reset_all(void)
+static void da_monitor_reset_all(void (*reset)(struct da_monitor *da_mon))
 {
        struct da_monitor_storage *mon_storage;
        int bkt;
 
        rcu_read_lock();
        hash_for_each_rcu(da_monitor_ht, bkt, mon_storage, node)
-               da_monitor_reset(&mon_storage->rv.da_mon);
+               reset(&mon_storage->rv.da_mon);
        rcu_read_unlock();
 }