From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7F9B9A0547; Fri, 25 Jun 2021 13:52:59 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0BD9C40698; Fri, 25 Jun 2021 13:52:59 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 2D29D4068A for ; Fri, 25 Jun 2021 13:52:57 +0200 (CEST) IronPort-SDR: vMB0i714KzBK07/+c7t46poDoZUcygJgg296PZsjS+CGSwbN88AbFMxdFv1pKH5CPMjbwrSv2b eX0dRwdJEtRw== X-IronPort-AV: E=McAfee;i="6200,9189,10025"; a="204646931" X-IronPort-AV: E=Sophos;i="5.83,298,1616482800"; d="scan'208";a="204646931" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jun 2021 04:52:56 -0700 IronPort-SDR: yoiGyNgXAfeP43k2kFWDakqweMlWQ4wDHHbyj69HNsCNemLxkAsuvqLGN3d9qWPKdSwxD0Hb56 81DbDjimldVA== X-IronPort-AV: E=Sophos;i="5.83,298,1616482800"; d="scan'208";a="455416394" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.213.251.3]) ([10.213.251.3]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jun 2021 04:52:54 -0700 To: "Ananyev, Konstantin" , "dev@dpdk.org" , "Hunt, David" Cc: "Loftus, Ciara" References: <85de3e30-eb1b-cd5c-5767-a2157d0d1616@intel.com> From: "Burakov, Anatoly" Message-ID: <6453df58-c5fd-49a5-f94b-fbe4516c4d9a@intel.com> Date: Fri, 25 Jun 2021 12:52:51 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 4/7] power: remove thread safety from PMD power API's X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 23-Jun-21 10:52 AM, Ananyev, Konstantin wrote: > > >> >> On 22-Jun-21 10:13 AM, Ananyev, Konstantin wrote: >>> >>>> Currently, we expect that only one callback can be active at any given >>>> moment, for a particular queue configuration, which is relatively easy >>>> to implement in a thread-safe way. However, we're about to add support >>>> for multiple queues per lcore, which will greatly increase the >>>> possibility of various race conditions. >>>> >>>> We could have used something like an RCU for this use case, but absent >>>> of a pressing need for thread safety we'll go the easy way and just >>>> mandate that the API's are to be called when all affected ports are >>>> stopped, and document this limitation. This greatly simplifies the >>>> `rte_power_monitor`-related code. >>> >>> I think you need to update RN too with that. >> >> Yep, will fix. >> >>> Another thing - do you really need the whole port stopped? >>> From what I understand - you work on queues, so it is enough for you >>> that related RX queue is stopped. >>> So, to make things a bit more robust, in pmgmt_queue_enable/disable >>> you can call rte_eth_rx_queue_info_get() and check queue state. >> >> We work on queues, but the data is per-lcore not per-queue, and it is >> potentially used by multiple queues, so checking one specific queue is >> not going to be enough. We could check all queues that were registered >> so far with the power library, maybe that'll work better? > > Yep, that's what I mean: on queue_enable() check is that queue stopped or not. > If not, return -EBUSY/EAGAIN or so/ > Sorry if I wasn't clear at first time. I think it's still better that all queues are stopped, rather than trying to work around the inherently racy implementation. So while i'll add the queue stopped checks, i'll still remove all of the thread safety stuff from here. -- Thanks, Anatoly