From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) by dpdk.org (Postfix) with ESMTP id 681F8568A for ; Sun, 26 Jul 2015 16:12:56 +0200 (CEST) Received: by wibxm9 with SMTP id xm9so80567478wib.0 for ; Sun, 26 Jul 2015 07:12:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=rJw88K/XkWbcROwahiWH1HDUY9iccg0H3yCfGo2rhW4=; b=gylf2CNYlChkhkKmbstHHJvDP3sUIK5mnXgyAeql24s6+Uwczny+Se5ZkZuXZyfRIn wZhUSFLJanVl7ntpcpd8eZ61ExUibxl/WGGfqgCl+/zHlfjto3JckRQ6azQHQM4HtxbQ h5/GBJdZnD9CDNozlb1Rhxj1wGN1B9QBtITqQjBiO9cGVuLSLUDBhchgz3QW65apgxW5 /n0+1l4ywbXZ28Ax/fDy+TDRKwt29bjYWzF3qOjKcC2urgdDqzxMuuCWnzdP8L4COi7q MnDqkavBYQzlBUC6jc/lE9DUx9jRuroR8I7n/zNIsr8BuiahwK02WcmLBBVaK3FfYWjU gjlA== X-Gm-Message-State: ALoCoQnoP3MRRF07f/lWWBcl6rPaxc5n35/PQTeiKI1fD4WxQJTXiSml3QxjbDDEwcd/c8Ws8iHZ X-Received: by 10.180.75.243 with SMTP id f19mr14741288wiw.52.1437919976209; Sun, 26 Jul 2015 07:12:56 -0700 (PDT) Received: from xps13.localnet (117.114.118.80.rev.sfr.net. [80.118.114.117]) by smtp.gmail.com with ESMTPSA id ej5sm22699572wjd.22.2015.07.26.07.12.54 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 26 Jul 2015 07:12:55 -0700 (PDT) From: Thomas Monjalon To: rsanford2@gmail.com Date: Sun, 26 Jul 2015 16:11:40 +0200 Message-ID: <4708068.jtpnxJe3Ir@xps13> Organization: 6WIND User-Agent: KMail/4.14.8 (Linux/4.0.4-2-ARCH; KDE/4.14.8; x86_64; ; ) In-Reply-To: <1437691347-58708-1-git-send-email-rsanford2@gmail.com> References: <1437691347-58708-1-git-send-email-rsanford2@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 0/3] timer: fix rte_timer_manage and improve unit tests X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 26 Jul 2015 14:12:56 -0000 2015-07-23 18:42, rsanford2@gmail.com: > From: Robert Sanford > > This patchset fixes a bug in timer stress test 2, adds a new stress test > to expose a race condition bug in API rte_timer_manage(), and then fixes > the rte_timer_manage() bug. > -- > > Patch 1, app/test timer stress test 2: Sometimes this test fails and > seg-faults because the slave lcores get out of phase with the master. > The master uses a single int, 'ready', to synchronize multiple slave > lcores through multiple phases of the test. > > To resolve, we construct simple synchronization primitives that use one > atomic-int state variable per slave. The master tells the slaves when to > start, and then waits for all of them to finish. Each slave waits for > the master to tell it to start, and then tells the master when it has > finished. > -- > > Description of rte_timer_manage() race condition bug: Through code > inspection, we noticed a potential problem in rte_timer_manage() that > leads to corruption of per-lcore pending-lists (implemented as > skip-lists). The race condition occurs when rte_timer_manage() expires > multiple timers on lcore A, while lcore B simultaneously invokes > rte_timer_reset() for one of the expiring timers (other than the first > one). > > Lcore A splits its pending-list, creating a local list of expired timers > linked through their sl_next[0] pointers, and sets the first expired > timer to the RUNNING state, all during one list-lock round trip. Lcore A > then unlocks the list-lock to run the first callback, and that is when > lcore B can misinterpret the subsequent expired timers' true states. > Lcore B sees an expired timer still in the PENDING state, locks A's > list-lock, and reinserts the timer into A's pending-list. We are trying > to use the same pointer to belong to both lists! > > One solution is to remove timers from the pending-list and set them to > the RUNNING state in one atomic step, i.e., we should perform these two > actions within one ownership of the list-lock. > -- > > Patch 2, new timer-manage race-condition test: We wrote a test to > confirm our suspicion that we could crash rte_timer_manage() (RTM) under > the right circumstances. We repeatedly set several timers to expire at > roughly the same time on the master core. The master lcore just delays > and runs RTM about ten times per second. The slave lcores all watch the > first timer (timer-0) to see when RTM is running on the master, i.e., > timer-0's state is not PENDING. At this point, each slave attempts to > reset a subset of the timers to a later expiration time. The goal here > is to have the slaves moving most of the timers to a different place in > the master's pending-list, while the master is working with the same > next-pointers and running callback functions. This eventually results in > the master traversing a corrupted linked-list. In our observations, it > results in an infinite loop. > -- > > Patch 3, eliminate race condition in rte_timer_manage(): After splitting > the pending-list at the current time point, we try to set all expired > timers to the RUNNING state, and put back into the pending-list any > timers that we fail to set to the RUNNING state, all during one > list-lock round trip. It is then safe to run the callback functions > for all expired timers that remain on our local list, without the lock. Please, could you re-send this serie after having added the description of each patch in the commit messages? It seems you fix 2 bugs in the first patch. It may be clearer to split it in 2 patches with separate explanations. Thanks