Re: [dpdk-dev] [21.08 PATCH v1 1/2] power: invert the monitor check

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

On 28-May-21 10:09 AM, Ananyev, Konstantin wrote:
> 
>>>
>>> On 25-May-21 10:15 AM, Liu, Yong wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Anatoly Burakov
>>>>> Sent: Tuesday, May 11, 2021 11:32 PM
>>>>> To: dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
>>> Xing,
>>>>> Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yang,
>>>>> Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>>>>> Wang, Haiyue <haiyue.wang@intel.com>; Matan Azrad
>>>>> <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Viacheslav
>>>>> Ovsiienko <viacheslavo@nvidia.com>; Richardson, Bruce
>>>>> <bruce.richardson@intel.com>; Ananyev, Konstantin
>>>>> <konstantin.ananyev@intel.com>
>>>>> Cc: Loftus, Ciara <ciara.loftus@intel.com>
>>>>> Subject: [dpdk-dev] [21.08 PATCH v1 1/2] power: invert the monitor check
>>>>>
>>>>> Previously, the semantics of power monitor were such that we were
>>>>> checking current value against the expected value, and if they matched,
>>>>> then the sleep was aborted. This is somewhat inflexible, because it only
>>>>> allowed us to check for a specific value.
>>>>>
>>>>> We can reverse the check, and instead have monitor sleep to be aborted
>>>>> if the expected value *doesn't* match what's in memory. This allows us
>>>>> to both implement all currently implemented driver code, as well as
>>>>> support more use cases which don't easily map to previous semantics
>>>>> (such as waiting on writes to AF_XDP counter value).
>>>>>
>>>>
>>>> Hi Anatoly,
>>>> In virtio spec, packed formatted descriptor utilizes two bits for representing
>>> the status. One bit for available status, one bit for used status.
>>>> For checking the status more precisely, it is need to check value against the
>>> expected value.
>>>> The monitor function in virtio datapath still can work with new semantics,
>>> but it may lead to some useless io call.
>>>> Base on that, I'd like to keep previous semantics.
>>>>
>>>> Regards,
>>>> Marvin
>>>>
>>>
>>> Thanks for your feedback! Would making this an option make things
>>> better? Because we need the inverted semantics for AF_XDP, it can't work
>>> without it. So, we either invert all of them, or we have an option to do
>>> regular or inverted check on a per-condition basis. Would that work?
>>>
>>
>> That will be great if we can select the check type based on input parameter.
>> Just in virtio datapath, we need both inverted and original semantics for different ring formats.
>>
> 
> Should we probably the consider introducing _check_ callback to be provided by PMD?
> So we can leave these various check details inside PMD itself.
> And monitor will just read the specified address and call the callback.
> Konstantin
> 

Getting monitor condition *is* "the check" IMO. I think adding an option 
to the comparison should cover pretty much all worthwhile use cases 
without overcomplicating things. In any case, patches already sent [1] :)

[1] http://patches.dpdk.org/project/dpdk/list/?series=17191

-- 
Thanks,
Anatoly