On 15/01/2021 13:33, Jan Beulich wrote: > 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?
Sounds good. My R-by stands with this change. ~Andrew
