On 13.03.2024 17:31, George Dunlap wrote:
> On Wed, Mar 13, 2024 at 2:00 PM Jan Beulich <[email protected]> wrote:
>>
>> When not holding the PoD lock across the entire region covering P2M
>> update and stats update, the entry count - if to be incorrect at all -
>> should indicate too large a value in preference to a too small one, to
>> avoid functions bailing early when they find the count is zero. However,
>> instead of moving the increment ahead (and adjust back upon failure),
>> extend the PoD-locked region.
>>
>> Fixes: 99af3cd40b6e ("x86/mm: Rework locking in the PoD layer")
>> Signed-off-by: Jan Beulich <[email protected]>
>
> Would you mind commenting on why you went with multiple unlocks,
> rather than multiple if statements?
Simply because what I did I view as more logical a code structure
than ...
> e.g.,
>
> ```
> rc = p2m_set_entry(...);
>
> /* Do the pod entry adjustment while holding the lock on success */
> if ( rc == 0 ) {
> /* adjust pod entries */
> }
>
> pod_unlock(p2m);
>
> /* Do the rest of the clean-up and error handling */
> if (rc == 0 ) {
... this, ...
> Just right now the multiple unlocks makes me worry that we may forget
> one at some point.
... despite this possible concern. But well, if going the other route
is what it takes to finally get this in, so be it.
Jan
