Re: Ping: [PATCH] Argo: don't obtain excess page references

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.01.2023 09:03, Jan Beulich wrote:
> On 30.01.2023 05:35, Christopher Clark wrote:
>> On Mon, Nov 21, 2022 at 4:41 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>>> On 11.10.2022 11:28, Jan Beulich wrote:
>>>> find_ring_mfn() already holds a page reference when trying to obtain a
>>>> writable type reference. We shouldn't make assumptions on the general
>>>> reference count limit being effectively "infinity". Obtain merely a type
>>>> ref, re-using the general ref by only dropping the previously acquired
>>>> one in the case of an error.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> Ping?
>>
>> Sorry it has taken me so long to review this patch and thank-you for
>> posting it. The points raised are helpful.
>>
>> Wrt to the patch - I can't ack because:
>> the general ref that is already held is from the page owner, and it may
>> actually be foreign; so the second ref acquire is currently ensuring that
>> it is a match for the owner of the ring. That needs addressing.
> 
> I'm afraid I may not understand your reply: Are you saying there's something
> wrong with the change? Or are you saying there's something wrong that merely
> becomes apparent due to the change? Or yet something else?

And to extend on the questions: What notion of "foreign" are you referring
to here? The earlier check_get_page_from_gfn() is passed the very same
domain. And anyway, get_page() wouldn't allow to acquire a ref with a
domain other than the page owner. Plus the sole caller of find_ring_mfns()
passes currd.

Jan