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 <[email protected]> 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 <[email protected]> >>> >>> 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
