From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 0AEC72965 for ; Mon, 29 Oct 2018 15:18:17 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 8DF472214E; Mon, 29 Oct 2018 10:18:17 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 29 Oct 2018 10:18:17 -0400 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=zM+jfcg7UZCxKR41orIjBlw+9V/SwC6X74RGDM3i0TY=; b=MbAMPSrw3WvP x5L4hP6GHFYZ4w5kk4qqslD4HrYycwkCUSim7ED/JN0WBRunRm/PGvaewfYLQRyJ kVbkKDC+Yc6J7jz4Sxk/sdiSyRwUkMzApdN7ZBvW0+nlMf5QG1WqqMLIaiPJexOU 8y/7Y/J6/+wygdhRd6Sbh4wZWKEqToY= 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=zM+jfcg7UZCxKR41orIjBlw+9V/SwC6X74RGDM3i0 TY=; b=xYCTunPrQVCPC9qsMxmHvf5JTP2J/pqJb7WWT9RbES8IA3nQT6qvpJmEO D2C2N2O1OwPurYzbkdbNIibtqb7Q9WGwQMS43FVQq3AJeDVKiMhyPGS2gc1P00p0 FbkZ6U0/w5HbZ2dV1P4ytQ2fN3CAnMyYeD6AqyhTkRo3NLGbMY0X/FD1SxRzfL22 XO1qxLxIZ2hKbMm7Ehn4uc6ULAuNIhM3Q7jpUJBiqsItscvJYF9lWQ0G0CTIa1cK APkc1znlO+7JaMPls3h4d8IpQ7MqsiOty/FBeB1ZURffV5eo8iSoN3QfApUQHLoF /aOgi9SGo5NQiMbA5Xt3l6XlWaAWw== X-ME-Sender: 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 1796B10335; Mon, 29 Oct 2018 10:18:16 -0400 (EDT) From: Thomas Monjalon To: Alejandro Lucero Cc: lei.a.yao@intel.com, dev , "Xu, Qian Q" , xueqin.lin@intel.com, "Burakov, Anatoly" , Ferruh Yigit Date: Mon, 29 Oct 2018 15:18:21 +0100 Message-ID: <3483377.PMXnpSGLS9@xps> In-Reply-To: References: <1538743527-8285-1-git-send-email-alejandro.lucero@netronome.com> <2DBBFF226F7CF64BAFCA79B681719D954502B94F@shsmsx102.ccr.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v3 0/6] use IOVAs check based on DMA mask 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: Mon, 29 Oct 2018 14:18:18 -0000 29/10/2018 14:40, Alejandro Lucero: > On Mon, Oct 29, 2018 at 1:18 PM Yao, Lei A wrote: > > *From:* Alejandro Lucero [mailto:alejandro.lucero@netronome.com] > > On Mon, Oct 29, 2018 at 11:46 AM Thomas Monjalon > > wrote: > > > > 29/10/2018 12:39, Alejandro Lucero: > > > I got a patch that solves a bug when calling rte_eal_dma_mask using the > > > mask instead of the maskbits. However, this does not solves the > > deadlock. > > > > The deadlock is a bigger concern I think. > > > > I think once the call to rte_eal_check_dma_mask uses the maskbits instead > > of the mask, calling rte_memseg_walk_thread_unsafe avoids the deadlock. > > > > Yao, can you try with the attached patch? > > > > Hi, Lucero > > > > This patch can fix the issue at my side. Thanks a lot > > for you quick action. > > Great! > > I will send an official patch with the changes. Please, do not forget my other request to better comment functions. > I have to say that I tested the patchset, but I think it was where > legacy_mem was still there and therefore dynamic memory allocation code not > used during memory initialization. > > There is something that concerns me though. Using > rte_memseg_walk_thread_unsafe could be a problem under some situations > although those situations being unlikely. > > Usually, calling rte_eal_check_dma_mask happens during initialization. Then > it is safe to use the unsafe function for walking memsegs, but with device > hotplug and dynamic memory allocation, there exists a potential race > condition when the primary process is allocating more memory and > concurrently a device is hotplugged and a secondary process does the device > initialization. By now, this is just a problem with the NFP, and the > potential race condition window really unlikely, but I will work on this > asap. Yes, this is what concerns me. You can add a comment explaining the unsafe which is not handled. > > > Interestingly, the problem looks like a compiler one. Calling > > > rte_memseg_walk does not return when calling inside rt_eal_dma_mask, > > but if > > > you modify the call like this: > > > > > > - if (rte_memseg_walk(check_iova, &mask)) > > > + if (!rte_memseg_walk(check_iova, &mask)) > > > > > > it works, although the value returned to the invoker changes, of course. > > > But the point here is it should be the same behaviour when calling > > > rte_memseg_walk than before and it is not. > > > > Anyway, the coding style requires to save the return value in a variable, > > instead of nesting the call in an "if" condition. > > And the "if" check should be explicitly != 0 because it is not a real > > boolean. > > > > PS: please do not top post and avoid HTML emails, thanks > > > > >