From patchwork Fri Jul 3 09:56:04 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ivan Khoronzhuk X-Patchwork-Id: 50620 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f70.google.com (mail-la0-f70.google.com [209.85.215.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id EE6A4214B3 for ; Fri, 3 Jul 2015 09:56:26 +0000 (UTC) Received: by laar3 with SMTP id r3sf26966159laa.1 for ; Fri, 03 Jul 2015 02:56:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:delivered-to:from:to:date :message-id:subject:precedence:list-id:list-unsubscribe:list-archive :list-post:list-help:list-subscribe:mime-version:content-type :content-transfer-encoding:errors-to:sender:x-original-sender :x-original-authentication-results:mailing-list; bh=6W1m8QtZww/9muxaQ1R18b3dAnKUny/zn3c330tY/cQ=; b=Omg7uEE2VApRiWWtMp6Ud6V3zT4YjInw9iCBtrSKfqdvpANTsP5WPuTOEDyazC6uvH 0W10Ridez2/jv0lFZWPoDnzhXEkjyu7I622ueBx1ERAMqP4rxO46P2fwfcbrTXRkV11w Xr20awgwG4U0gQCYB/S5FaohbptHnos5uCqmw0bCtfONyi5BPio4m5ksEWBg38DI1X5H Qgfb6vnIM6woVMhzfVh0eLNBS2+a3C4fqwaWxPH5IDX0RxnZ0dPZSyEYemKt/BngcmSh C9kN1CzPrmGoaz+hdhI8CaHbqmrgVHwm/tkwW9TpSGdDZyACYBr+RqsktLwtnXFXBozA /Yxw== X-Gm-Message-State: ALoCoQnzmUULj/OsV0R0V1R3Zeb/MVMiIssFLIs9plGD6kCdHzQNJNhRjISRHa2qFgmM8GNsJQ5Q X-Received: by 10.112.40.45 with SMTP id u13mr22293818lbk.0.1435917385875; Fri, 03 Jul 2015 02:56:25 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.5.1 with SMTP id o1ls474810lao.12.gmail; Fri, 03 Jul 2015 02:56:25 -0700 (PDT) X-Received: by 10.112.180.201 with SMTP id dq9mr1006851lbc.78.1435917385464; Fri, 03 Jul 2015 02:56:25 -0700 (PDT) Received: from mail-la0-f52.google.com (mail-la0-f52.google.com. [209.85.215.52]) by mx.google.com with ESMTPS id pl9si6790137lbb.111.2015.07.03.02.56.25 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Jul 2015 02:56:25 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.52 as permitted sender) client-ip=209.85.215.52; Received: by lagx9 with SMTP id x9so79110559lag.1 for ; Fri, 03 Jul 2015 02:56:25 -0700 (PDT) X-Received: by 10.152.26.163 with SMTP id m3mr35025938lag.86.1435917384983; Fri, 03 Jul 2015 02:56:24 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.108.230 with SMTP id hn6csp1055940lbb; Fri, 3 Jul 2015 02:56:23 -0700 (PDT) X-Received: by 10.140.41.134 with SMTP id z6mr48845634qgz.40.1435917383419; Fri, 03 Jul 2015 02:56:23 -0700 (PDT) Received: from lists.linaro.org (lists.linaro.org. [54.225.227.206]) by mx.google.com with ESMTP id 92si9799606qgg.18.2015.07.03.02.56.22; Fri, 03 Jul 2015 02:56:23 -0700 (PDT) Received-SPF: pass (google.com: domain of lng-odp-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) client-ip=54.225.227.206; Received: by lists.linaro.org (Postfix, from userid 109) id 8C90062045; Fri, 3 Jul 2015 09:56:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on ip-10-142-244-252.ec2.internal X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, URIBL_BLOCKED autolearn=disabled version=3.4.0 Received: from ip-10-142-244-252.ec2.internal (localhost [127.0.0.1]) by lists.linaro.org (Postfix) with ESMTP id 0523161DF6; Fri, 3 Jul 2015 09:56:18 +0000 (UTC) X-Original-To: lng-odp@lists.linaro.org Delivered-To: lng-odp@lists.linaro.org Received: by lists.linaro.org (Postfix, from userid 109) id 256DC6203C; Fri, 3 Jul 2015 09:56:15 +0000 (UTC) Received: from mail-la0-f41.google.com (mail-la0-f41.google.com [209.85.215.41]) by lists.linaro.org (Postfix) with ESMTPS id 126CD61DEE for ; Fri, 3 Jul 2015 09:56:14 +0000 (UTC) Received: by lagc2 with SMTP id c2so79167577lag.3 for ; Fri, 03 Jul 2015 02:56:12 -0700 (PDT) X-Received: by 10.152.36.102 with SMTP id p6mr35011599laj.19.1435917372344; Fri, 03 Jul 2015 02:56:12 -0700 (PDT) Received: from khorivan.synapse.com ([195.238.92.128]) by mx.google.com with ESMTPSA id i1sm2192430lae.24.2015.07.03.02.56.10 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 03 Jul 2015 02:56:11 -0700 (PDT) From: Ivan Khoronzhuk To: lng-odp@lists.linaro.org Date: Fri, 3 Jul 2015 12:56:04 +0300 Message-Id: <1435917364-22865-1-git-send-email-ivan.khoronzhuk@linaro.org> X-Mailer: git-send-email 1.9.1 X-Topics: timers patch Subject: [lng-odp] [Patch] example: timer: delete races while termination X-BeenThere: lng-odp@lists.linaro.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , MIME-Version: 1.0 Errors-To: lng-odp-bounces@lists.linaro.org Sender: "lng-odp" X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: ivan.khoronzhuk@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.52 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 Current implementation has at least two races that lead to several issues: - gbls->remain can overflow. One thread can decrement remain counter to 0. While another can decrement it once again and it will be > 0 once again. After what the thread will loop very long time ... - Several threads can terminate the same timer and as result the same event. After out from the main loop the thread terminates the last timer it used. But the last timer saved in ttp for one thread can be received in another thread. So after leaving the main loop two threads can hold the same timer. - Some timer cannot be freed as several threads try to delete the same timer, as result one of the timer/tmo stay not freed after termination. - The test can send more events that requested. The receiving of requested number of tmos doesn't mean the test send the same number. It rather sends more. This patch is intended to fix above drawbacks. The termination patch must follow the next things: - The event can be in the following places: in the timer (waiting to be scheduled), in the some queue for some thread to be scheduled, received in the main loop. - each event "holds" the timer, so when we receive event we can delete timer. - the thread cannot delete timer w/o event as it doesn't know who is owner of this event (and obvious the timer). - the thread shouldn't send events more than requested. - all threads have to be "held" in the loop till the last received event. The scheduler can assign event for any of the threads, so one thread can receive two last events for example. According to the above, added several improvements: - don't send more timeouts that supposed to receive - free timer and tmo for the last received tmos = num of threads. - leave the main loop only if the last tmo/timer is free. Signed-off-by: Ivan Khoronzhuk --- example/timer/odp_timer_test.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c index 5e4306e..e832e35 100644 --- a/example/timer/odp_timer_test.c +++ b/example/timer/odp_timer_test.c @@ -47,6 +47,7 @@ typedef struct { odp_timer_pool_t tp; /**< Timer pool handle*/ odp_atomic_u32_t remain; /**< Number of timeouts to receive*/ struct test_timer tt[256]; /**< Array of all timer helper structs*/ + int num_workers; /**< Number of threads */ } test_globals_t; /** @private Timer set status ASCII strings */ @@ -139,16 +140,18 @@ static void test_abs_timeouts(int thr, test_globals_t *gbls) ttp->ev = odp_timeout_to_event(tmo); tick = odp_timer_current_tick(gbls->tp); - while ((int)odp_atomic_load_u32(&gbls->remain) > 0) { + while (1) { odp_event_t ev; odp_timer_set_t rc; - tick += period; - rc = odp_timer_set_abs(ttp->tim, tick, &ttp->ev); - if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) { - /* Too early or too late timeout requested */ - EXAMPLE_ABORT("odp_timer_set_abs() failed: %s\n", - timerset2str(rc)); + if (ttp) { + tick += period; + rc = odp_timer_set_abs(ttp->tim, tick, &ttp->ev); + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) { + /* Too early or too late timeout requested */ + EXAMPLE_ABORT("odp_timer_set_abs() failed: %s\n", + timerset2str(rc)); + } } /* Get the next expired timeout. @@ -185,18 +188,17 @@ static void test_abs_timeouts(int thr, test_globals_t *gbls) } EXAMPLE_DBG(" [%i] timeout, tick %"PRIu64"\n", thr, tick); - odp_atomic_dec_u32(&gbls->remain); - } + int rx_num = odp_atomic_fetch_dec_u32(&gbls->remain); + if (!rx_num) + EXAMPLE_ABORT("Unexpected timeout received (timer %x, tick %"PRIu64")\n", + ttp->tim, tick); + else if (rx_num > gbls->num_workers) + continue; - /* Cancel and free last timer used */ - (void)odp_timer_cancel(ttp->tim, &ttp->ev); - if (ttp->ev != ODP_EVENT_INVALID) odp_timeout_free(odp_timeout_from_event(ttp->ev)); - else - EXAMPLE_ERR("Lost timeout event at timer cancel\n"); - /* Since we have cancelled the timer, there is no timeout event to - * return from odp_timer_free() */ - (void)odp_timer_free(ttp->tim); + odp_timer_free(ttp->tim); + ttp = NULL; + } /* Remove any prescheduled events */ remove_prescheduled_events(); @@ -483,6 +485,8 @@ int main(int argc, char *argv[]) printf("\n"); + gbls->num_workers = num_workers; + /* Initialize number of timeouts to receive */ odp_atomic_init_u32(&gbls->remain, gbls->args.tmo_count * num_workers);