service: fix race in service on app lcore function

[dpdk.git] / lib / librte_eal / common / rte_service.c

diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index f17bf4b..09b758c 100644 (file)
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -77,7 +77,7 @@ struct rte_service_spec_impl {
        uint8_t internal_flags;
 
        /* per service statistics */
-       uint32_t num_mapped_cores;
+       rte_atomic32_t num_mapped_cores;
        uint64_t calls;
        uint64_t cycles_spent;
 } __rte_cache_aligned;
@@ -325,7 +325,7 @@ rte_service_runstate_get(uint32_t id)
        rte_smp_rmb();
 
        int check_disabled = !(s->internal_flags & SERVICE_F_START_CHECK);
-       int lcore_mapped = (s->num_mapped_cores > 0);
+       int lcore_mapped = (rte_atomic32_read(&s->num_mapped_cores) > 0);
 
        return (s->app_runstate == RUNSTATE_RUNNING) &&
                (s->comp_runstate == RUNSTATE_RUNNING) &&
@@ -365,7 +365,7 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask)
         * mapped, atomic ops are not required.
         */
        const int use_atomics = (service_mt_safe(s) == 0) &&
-                               (s->num_mapped_cores > 1);
+                               (rte_atomic32_read(&s->num_mapped_cores) > 1);
        if (use_atomics) {
                if (!rte_atomic32_cmpset((uint32_t *)&s->execute_lock, 0, 1))
                        return -EBUSY;
@@ -378,11 +378,36 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask)
        return 0;
 }
 
-int32_t rte_service_run_iter_on_app_lcore(uint32_t id)
+int32_t rte_service_run_iter_on_app_lcore(uint32_t id,
+               uint32_t serialize_mt_unsafe)
 {
        /* run service on calling core, using all-ones as the service mask */
+       if (!service_valid(id))
+               return -EINVAL;
+
        struct core_state *cs = &lcore_states[rte_lcore_id()];
-       return service_run(id, cs, UINT64_MAX);
+       struct rte_service_spec_impl *s = &rte_services[id];
+
+       /* Atomically add this core to the mapped cores first, then examine if
+        * we can run the service. This avoids a race condition between
+        * checking the value, and atomically adding to the mapped count.
+        */
+       if (serialize_mt_unsafe)
+               rte_atomic32_inc(&s->num_mapped_cores);
+
+       if (service_mt_safe(s) == 0 &&
+                       rte_atomic32_read(&s->num_mapped_cores) > 1) {
+               if (serialize_mt_unsafe)
+                       rte_atomic32_dec(&s->num_mapped_cores);
+               return -EBUSY;
+       }
+
+       int ret = service_run(id, cs, UINT64_MAX);
+
+       if (serialize_mt_unsafe)
+               rte_atomic32_dec(&s->num_mapped_cores);
+
+       return ret;
 }
 
 static int32_t
@@ -522,10 +547,10 @@ service_update(struct rte_service_spec *service, uint32_t lcore,
        if (set) {
                if (*set) {
                        lcore_states[lcore].service_mask |= sid_mask;
-                       rte_services[sid].num_mapped_cores++;
+                       rte_atomic32_inc(&rte_services[sid].num_mapped_cores);
                } else {
                        lcore_states[lcore].service_mask &= ~(sid_mask);
-                       rte_services[sid].num_mapped_cores--;
+                       rte_atomic32_dec(&rte_services[sid].num_mapped_cores);
                }
        }
 
@@ -568,7 +593,7 @@ int32_t rte_service_lcore_reset_all(void)
                lcore_states[i].runstate = RUNSTATE_STOPPED;
        }
        for (i = 0; i < RTE_SERVICE_NUM_MAX; i++)
-               rte_services[i].num_mapped_cores = 0;
+               rte_atomic32_set(&rte_services[i].num_mapped_cores, 0);
 
        rte_smp_wmb();
 
@@ -664,7 +689,8 @@ rte_service_lcore_stop(uint32_t lcore)
        for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) {
                int32_t enabled = service_mask & (UINT64_C(1) << i);
                int32_t service_running = rte_service_runstate_get(i);
-               int32_t only_core = rte_services[i].num_mapped_cores == 1;
+               int32_t only_core = (1 ==
+                       rte_atomic32_read(&rte_services[i].num_mapped_cores));
 
                /* if the core is mapped, and the service is running, and this
                 * is the only core that is mapped, the service would cease to