Re: [Xen-devel] [PATCH 2/2] RFC: x86/mm: conditionally check page_lock/page_unlock ownership

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Fri, Apr 12, 2019 at 6:01 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> >>> On 12.04.19 at 06:29, <tamas@xxxxxxxxxxxxx> wrote:
> > Patch cf4b30dca0a "Add debug code to detect illegal page_lock and 
> > put_page_type
> > ordering" added extra sanity checking to page_lock/page_unlock for debug 
> > builds
> > with the assumption that no hypervisor path ever locks two pages at once.
> >
> > This assumption doesn't hold during memory sharing.
>
> A fundamental question is - doesn't mem-sharing abuse page_lock()?
> I've never understood why it uses it, and back when it was introduced
> the function clearly wasn't meant to be used outside of PV memory
> management code.

I have limited understanding of what the limitations are of page_lock
but here is the reasoning for taking the lock for two pages.
Mem_sharing takes the pages it wants to be sharable and assigns them
to dom_cow at least with one reference being held (since we just took
these pages from a live domain). Those pages may then be assigned to
several actual domains at the same time, and each domain could be
destroyed at any given point. There is no requirement that the
original domain still be alive since the owner of these pages is now
dom_cow. The backing pages owned by dom_cow only get dropped when the
reference count hits zero, ie. no actual domain actually uses those
pages anymore. This could happen if all domains unshare the gfn's that
use this page as the backing or if all domains that use them for
mem_sharing get destroyed. When we share pages, we want to make sure
those don't get dropped under our feet while we are in the middle of
this function. So we lock both pages.

Also, I don't see why it would be fundamentally wrong to hold the
locks to two pages at once. Is this a problem with the lock
implementation itself?

>
> One minimal remark on the patch itself: Please use true/false instead
> of 1/0 when passing bool arguments.

Ack.

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel