On 4/26/19 6:21 PM, Tamas K Lengyel wrote: > Calling _put_page_type while also holding the page_lock > for that page can cause a deadlock.
I can't immediately see what the mem_sharing_page_[un]lock() functions are meant to be protecting against. Is there a comment anywhere describing what they're used for or how they work? Because... > Signed-off-by: Tamas K Lengyel <ta...@tklengyel.com> > Cc: Jan Beulich <jbeul...@suse.com> > Cc: Andrew Cooper <andrew.coop...@citrix.com> > Cc: George Dunlap <george.dun...@eu.citrix.com> > Cc: Wei Liu <wei.l...@citrix.com> > Cc: Roger Pau Monne <roger....@citrix.com> > --- > v3: simplified patch by keeping the additional references already in-place > --- > xen/arch/x86/mm/mem_sharing.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c > index dfc279d371..e2f74ac770 100644 > --- a/xen/arch/x86/mm/mem_sharing.c > +++ b/xen/arch/x86/mm/mem_sharing.c > @@ -648,10 +648,6 @@ static int page_make_private(struct domain *d, struct > page_info *page) > return -EBUSY; > } > > - /* We can only change the type if count is one */ > - /* Because we are locking pages individually, we need to drop > - * the lock here, while the page is typed. We cannot risk the > - * race of page_unlock and then put_page_type. */ ...the comment you're removing explicitly says that what you're doing is "risk"-y.; but you don't explain at all why the comment is wrong. Ultimately you're the maintainer, and this is non-security-supported, so if you insist it's safe, I won't oppose it; but it seems like having some clarity about what is or is not risky and why would be helpful. -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
