Andres Lagar-Cavilla
2013-Aug-06 15:38 UTC
Re: mem_sharing_nominate_page: p2mt should never change before p2m_change_type()
> The p2mt change check for p2m_change_type() was first introduced when this code path was not protected by p2m_lock(). > Now this code path is protected by p2m_lock. So p2mt should never change before p2m_change_type().Makes sense. You need to put a proper Signed-off-by line. In any case: Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org> Andres> > --- > diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c > index 0364bb0..b3940cb 100644 > --- a/xen/arch/x86/mm/mem_sharing.c > +++ b/xen/arch/x86/mm/mem_sharing.c > @@ -900,20 +900,8 @@ int mem_sharing_nominate_page(struct domain *d, > goto out; > } > > - /* Change the p2m type */ > - if ( p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt ) > - { > - /* This is unlikely, as the type must have changed since we''ve checked > - * it a few lines above. > - * The mfn needs to revert back to rw type. This should never fail, > - * since no-one knew that the mfn was temporarily sharable */ > - mem_sharing_gfn_destroy(page, d, gfn_info); > - xfree(page->sharing); > - page->sharing = NULL; > - /* NOTE: We haven''t yet added this to the audit list. */ > - BUG_ON(page_make_private(d, page) != 0); > - goto out; > - } > + /* Change the p2m type, should never fail with p2m locked. */ > + BUG_ON(p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt); > > /* Account for this page. */ > atomic_inc(&nr_shared_mfns); > > >
Nai Xia
2013-Aug-06 16:25 UTC
Re: mem_sharing_nominate_page: p2mt should never change before p2m_change_type()
Tim Deegan
2013-Aug-08 09:49 UTC
Re: mem_sharing_nominate_page: p2mt should never change before p2m_change_type()
At 00:25 +0800 on 07 Aug (1375835148), Nai Xia wrote:> Thanks. Please append the following line to the change log: > Signed-off-by: Nai Xia <nai.xia@gmail.com>Done, acked + applied. Cheers, Tim.> BR, > Nai Xia > > On 2013???08???06??? 23:38, Andres Lagar-Cavilla wrote: > >> The p2mt change check for p2m_change_type() was first introduced when this code path was not protected by p2m_lock(). > >> Now this code path is protected by p2m_lock. So p2mt should never change before p2m_change_type(). > > > > Makes sense. You need to put a proper Signed-off-by line. In any case: > > Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org> > > > > Andres > >> > >> --- > >> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c > >> index 0364bb0..b3940cb 100644 > >> --- a/xen/arch/x86/mm/mem_sharing.c > >> +++ b/xen/arch/x86/mm/mem_sharing.c > >> @@ -900,20 +900,8 @@ int mem_sharing_nominate_page(struct domain *d, > >> goto out; > >> } > >> > >> - /* Change the p2m type */ > >> - if ( p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt ) > >> - { > >> - /* This is unlikely, as the type must have changed since we''ve checked > >> - * it a few lines above. > >> - * The mfn needs to revert back to rw type. This should never fail, > >> - * since no-one knew that the mfn was temporarily sharable */ > >> - mem_sharing_gfn_destroy(page, d, gfn_info); > >> - xfree(page->sharing); > >> - page->sharing = NULL; > >> - /* NOTE: We haven''t yet added this to the audit list. */ > >> - BUG_ON(page_make_private(d, page) != 0); > >> - goto out; > >> - } > >> + /* Change the p2m type, should never fail with p2m locked. */ > >> + BUG_ON(p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt); > >> > >> /* Account for this page. */ > >> atomic_inc(&nr_shared_mfns); > >> > >> > >> > >> pub 1024D/E6BAA6F9 2008-06-09 Nai Xia <xianai@nju.edu.cn> > sub 2048g/8AA69835 2008-06-09> _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel