Re: [dpdk-dev] [PATCH v1 4/7] power: remove thread safety from PMD power API's

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

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