Re: [PATCH 2/3] gnttab: consolidate pin-to-status syncing

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.01.2021 14:25, Andrew Cooper wrote:
> On 14/01/2021 15:23, Jan Beulich wrote:
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -908,6 +908,25 @@ static int _set_status(const grant_entry
>>          return _set_status_v2(shah, status, rd, act, readonly, mapflag, 
>> ldomid);
>>  }
>>  
>> +/*
>> + * The status for a grant may indicate that we're taking more access than
>> + * the pin requires.  Fix up the status to match the pin.
> 
> This sentence isn't correct.  It will only clear status flags to match a
> reduction in the pinned references.  IOW it cannot be used in the case
> that a grant goes from unpinned to pinned.

Of course not, hence "... more access than ...". But yes, I can
replace "Fix up" by "Reduce" if you think the wording isn't
unambiguous enough.

> How about renaming to clear_status_for_pin() to make this clearer?  I
> don't think it is worth trying to make the operation more generic.

Hmm, this name would suggest to me that the function clears
whatever the present pin count requires (e.g. acting on the
pre-update value when the post-update one is [going to be]
zero). Maybe reduce_status_for_pin(), matching the suggested
comment wording change?

> If so (and with a suitable adjustment to the comment), Reviewed-by:
> Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

As per above, I'll first wait for your further reply.

Jan