On Wed, Dec 18, 2019 at 2:29 PM Julien Grall wrote:
>
> Hi Tamas,
>
> On 18/12/2019 19:40, Tamas K Lengyel wrote:
> > MEM_SHARING_DESTROY_GFN is used on the 'flags' bitfield during unsharing.
> > However, the bitfield is not used for anything else, so just convert it to a
> > bool instead.
> >
> > Signed-off-by: Tamas K Lengyel
> > ---
> > xen/arch/x86/mm/mem_sharing.c | 7 +++
> > xen/arch/x86/mm/p2m.c | 1 +
> > xen/common/memory.c | 2 +-
> > xen/include/asm-x86/mem_sharing.h | 5 ++---
> > 4 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> > index fc1d8be1eb..6e81e1a895 100644
> > --- a/xen/arch/x86/mm/mem_sharing.c
> > +++ b/xen/arch/x86/mm/mem_sharing.c
> > @@ -1175,7 +1175,7 @@ err_out:
> >*/
> > int __mem_sharing_unshare_page(struct domain *d,
> > unsigned long gfn,
> > - uint16_t flags)
> > + bool destroy)
> > {
> > p2m_type_t p2mt;
> > mfn_t mfn;
> > @@ -1231,7 +1231,7 @@ int __mem_sharing_unshare_page(struct domain *d,
> >* If the GFN is getting destroyed drop the references to MFN
> >* (possibly freeing the page), and exit early.
> >*/
> > -if ( flags & MEM_SHARING_DESTROY_GFN )
> > +if ( destroy )
> > {
> > if ( !last_gfn )
> > mem_sharing_gfn_destroy(page, d, gfn_info);
> > @@ -1321,8 +1321,7 @@ int relinquish_shared_pages(struct domain *d)
> > if ( mfn_valid(mfn) && p2m_is_shared(t) )
> > {
> > /* Does not fail with ENOMEM given the DESTROY flag */
> > -BUG_ON(__mem_sharing_unshare_page(d, gfn,
> > - MEM_SHARING_DESTROY_GFN));
> > +BUG_ON(__mem_sharing_unshare_page(d, gfn, true));
> > /*
> >* Clear out the p2m entry so no one else may try to
> >* unshare. Must succeed: we just read the old entry and
> > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> > index baea632acc..53ea44fe3c 100644
> > --- a/xen/arch/x86/mm/p2m.c
> > +++ b/xen/arch/x86/mm/p2m.c
> > @@ -517,6 +517,7 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m,
> > unsigned long gfn_l,
> >*/
> > if ( mem_sharing_unshare_page(p2m->domain, gfn_l) < 0 )
> > mem_sharing_notify_enomem(p2m->domain, gfn_l, false);
> > +
>
> This line looks spurious.
Yeap.
>
> > mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL);
> > }
> >
> > diff --git a/xen/common/memory.c b/xen/common/memory.c
> > index 309e872edf..c7d2bac452 100644
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -352,7 +352,7 @@ int guest_remove_page(struct domain *d, unsigned long
> > gmfn)
> >* might be the only one using this shared page, and we need to
> >* trigger proper cleanup. Once done, this is like any other page.
> >*/
> > -rc = mem_sharing_unshare_page(d, gmfn, 0);
> > +rc = mem_sharing_unshare_page(d, gmfn);
>
> AFAICT, this patch does not reduce the number of parameters for
> mem_sharing_unshare_page(). Did you intend to make this change in
> another patch?
Ah yea, it should have been dropped in patch 6 of the series.
>
> > if ( rc )
> > {
> > mem_sharing_notify_enomem(d, gmfn, false);
> > diff --git a/xen/include/asm-x86/mem_sharing.h
> > b/xen/include/asm-x86/mem_sharing.h
> > index 89cdaccea0..4b982a4803 100644
> > --- a/xen/include/asm-x86/mem_sharing.h
> > +++ b/xen/include/asm-x86/mem_sharing.h
> > @@ -76,17 +76,16 @@ struct page_sharing_info
> > unsigned int mem_sharing_get_nr_saved_mfns(void);
> > unsigned int mem_sharing_get_nr_shared_mfns(void);
> >
> > -#define MEM_SHARING_DESTROY_GFN (1<<1)
> > /* Only fails with -ENOMEM. Enforce it with a BUG_ON wrapper. */
> > int __mem_sharing_unshare_page(struct domain *d,
> > unsigned long gfn,
> > - uint16_t flags);
> > + bool destroy);
> >
> > static inline
> > int mem_sharing_unshare_page(struct domain *d,
> >unsigned long gfn)
> > {
> > -int rc = __mem_sharing_unshare_page(d, gfn, 0);
> > +int rc = __mem_sharing_unshare_page(d, gfn, false);
> > BUG_ON(rc && (rc != -ENOMEM));
> > return rc;
> > }
> >
>
> Cheers,
Thanks,
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
