From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 2F1691B59F; Wed, 19 Dec 2018 19:54:18 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id B6447221AF; Wed, 19 Dec 2018 13:54:17 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 19 Dec 2018 13:54:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=Wb9DbvUCRFMW07W8FafB45f8AZ+bdNEdTdklEjvzTbU=; b=M98PsfFPxNNF 0sd/SAajkW/IildQMThZfjGg+cbFDgyopr5k4Rozpk+9HwetQwqoJ0EEsJq0SpDR wAraH5FXiC+xG0HNZ2BtGfKRn3/zOiqmLnvPyGoma5uI0hEkxvn2UyIKWm/nbB5+ hGnFfPa2NCIRX70tnSqXOdfZS6VjT5Y= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=Wb9DbvUCRFMW07W8FafB45f8AZ+bdNEdTdklEjvzT bU=; b=SjvyGWBm2iBxQyj2ioh2nNYlgK0Bwra7UKHkxcXVH/bA8uMynumsFBHcQ mMWDHd2om3X5RzMcGAGWwJbFTGHZjQMTwX+OgvMjsUmmG90j9dlIOcXPGecrrAn/ Ww1zdfNTJeNGymx3uHgnHmHXMgwzK3yaYqb76boVnvZrKK7HA2HAxRM48hZ1Wg/Z jgze0yvz/ot3/1bHDzrBpG+Zhd0EVQDdfsF4smrnmc2QoKn8q+v/v4nT6jI4/58s eZENg7LusqqggghgWRjJNZ/gm4VWDZeXHERDp94vxo/x17yyx4mxz2tplz1Wo0Dt ITwdgp8MQoUYIifuIZb6WNLltWKdw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedtkedrudejtddguddulecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfhuthenuceurghilhhouhhtmecu fedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujfgurhephffvufffkf gjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgrshcuofhonhhjrghlohhn uceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecukfhppeejjedrudefgedrvd dtfedrudekgeenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgr lhhonhdrnhgvthenucevlhhushhtvghrufhiiigvpedt X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 1E004E405D; Wed, 19 Dec 2018 13:54:16 -0500 (EST) From: Thomas Monjalon To: Erik Gabriel Carrillo Cc: dev@dpdk.org, Gavin.Hu@arm.com, rsanford@akamai.com, olivier.matz@6wind.com, stephen@networkplumber.org, bruce.richardson@intel.com, stable@dpdk.org Date: Wed, 19 Dec 2018 19:54:15 +0100 Message-ID: <1710260.bNaUKXAX4B@xps> In-Reply-To: <1545235774-9834-1-git-send-email-erik.g.carrillo@intel.com> References: <1543517626-142526-1-git-send-email-erik.g.carrillo@intel.com> <1545235774-9834-1-git-send-email-erik.g.carrillo@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2] timer: fix race condition X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Dec 2018 18:54:18 -0000 19/12/2018 17:09, Erik Gabriel Carrillo: > rte_timer_manage() adds expired timers to a "run list", and walks the > list, transitioning each timer from the PENDING to the RUNNING state. > If another lcore resets or stops the timer at precisely this > moment, the timer state would instead be set to CONFIG by that other > lcore, which would cause timer_manage() to skip over it. This is > expected behavior. > > However, if a timer expires quickly enough, there exists the > following race condition that causes the timer_manage() routine to > misinterpret a timer in CONFIG state, resulting in lost timers: > > - Thread A: > - starts a timer with rte_timer_reset() > - the timer is moved to CONFIG state > - the spinlock associated with the appropriate skiplist is acquired > - timer is inserted into the skiplist > - the spinlock is released > - Thread B: > - executes rte_timer_manage() > - find above timer as expired, add it to run list > - walk run list, see above timer still in CONFIG state, unlink it from > run list and continue on > - Thread A: > - move timer to PENDING state > - return from rte_timer_reset() > - timer is now in PENDING state, but not actually linked into a > pending list or a run list and will never get processed further > by rte_timer_manage() > > This commit fixes this race condition by only releasing the spinlock > after the timer state has been transitioned from CONFIG to PENDING, > which prevents rte_timer_manage() from seeing an incorrect state. > > Fixes: 9b15ba895b9f ("timer: use a skip list") > Signed-off-by: Erik Gabriel Carrillo > Reviewed-by: Gavin Hu +Cc: stable@dpdk.org Applied, thanks