Re: [Xen-devel] [PATCH 12 of 18] x86/mm: Make page_lock/unlock() in arch/x86/mm.c externally callable

[Tim Deegan <tim@xxxxxxx>]

At 18:54 -0800 on 08 Dec (1323370449), Andres Lagar-Cavilla wrote:
> > At 02:47 -0500 on 08 Dec (1323312447), Andres Lagar-Cavilla wrote:
> >> This is necessary for a new consumer of page_lock/unlock to follow in
> >> the series.
> >>
> >> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
> >
> > Nak, I'm afraid.
> >
> > These were OK as local functions but if they're going to be made
> > generally visible, they need clear comments describing what this
> > locking protects and what the discipline is for avoiding deadlocks.
> 
> How about something along the lines of
> "page_lock() is used for two purposes: pte serialization, and memory
> sharing. All users of page lock for pte serialization live in mm.c, use it
> to lock a page table page during pte updates, do not take other locks
> within the critical section delimited by page_lock/unlock, and perform no
> nesting. All users of page lock for memory sharing live in
> mm/mem_sharing.c. For memory sharing, nesting may happen when sharing (and
> locking) two pages -- deadlock is avoided by locking pages in increasing
> order. Memory sharing may take the p2m_lock within a page_lock/unlock
> critical section. These two users (pte serialization and memory sharing)
> should never collide, as long as page table pages are properly unshared
> prior to updating."

Thanks.  Extracting from that and from Keir's response: 

It serves both as a spinlock and to exclude any to the page-type of the
page in question (by causing the get_page_type() functions to spin).

What it currently protects is all modifications to pages that have
pagetable type.  This serialises PV PTE validations, both against other
validations of the same PTE and against concurrent changes of the
enclosing page's type.

Your planned use is to protect updates to the page-sharing state
associated with a page.  Can you be clear about what exactly is protected?

The proposed locking discipline is that
- page locks must be taken in increasing order of MFN, yes?  
- and that you must always take page locks before the p2m lock?

Is that about right?

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel