mem-sharing: move (x86) / drop (Arm) arch_dump_shared_mem_info()

[PATCH] mem-sharing: move (x86) / drop (Arm) arch_dump_shared_mem_info()

Posted by Jan Beulich 8 months, 4 weeks ago

When !MEM_SHARING no useful output is produced. Move the function into
mm/mem_sharing.c while conditionalizing the call to it, thus allowing to
drop it altogether from Arm (and eliminating the need to introduce stubs
on PPC and RISC-V).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I wasn't really sure whether introducing a stub in xen/mm.h would be any
better than adding the (further) #ifdef to dump_domains().

We could go further and also eliminate the need for the stub variants
of mem_sharing_get_nr_{shared,saved}_mfns() by moving the
XENMEM_get_sharing_{shared,freed}_pages cases in
{,compat_}arch_memory_op() into the already existing #ifdef-s there.
Returning an error for those sub-ops may be slightly more appropriate
than returning 0 when !MEM_SHARING.

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1297,10 +1297,6 @@ void free_init_memory(void)
     printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
 }
 
-void arch_dump_shared_mem_info(void)
-{
-}
-
 int steal_page(
     struct domain *d, struct page_info *page, unsigned int memflags)
 {
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -6265,13 +6265,6 @@ void memguard_unguard_stack(void *p)
     map_pages_to_xen((unsigned long)p, virt_to_mfn(p), 1, PAGE_HYPERVISOR_RW);
 }
 
-void arch_dump_shared_mem_info(void)
-{
-    printk("Shared frames %u -- Saved frames %u\n",
-            mem_sharing_get_nr_shared_mfns(),
-            mem_sharing_get_nr_saved_mfns());
-}
-
 const struct platform_bad_page *__init get_platform_badpages(unsigned int *array_size)
 {
     u32 igd_id;
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -2329,3 +2329,10 @@ int mem_sharing_domctl(struct domain *d,
 
     return rc;
 }
+
+void arch_dump_shared_mem_info(void)
+{
+    printk("Shared frames %u -- Saved frames %u\n",
+            mem_sharing_get_nr_shared_mfns(),
+            mem_sharing_get_nr_saved_mfns());
+}
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -365,7 +365,9 @@ static void cf_check dump_domains(unsign
         }
     }
 
+#ifdef CONFIG_MEM_SHARING
     arch_dump_shared_mem_info();
+#endif
 
     rcu_read_unlock(&domlist_read_lock);
 }

Ping: [PATCH] mem-sharing: move (x86) / drop (Arm) arch_dump_shared_mem_info()

Posted by Jan Beulich 8 months, 2 weeks ago

On 08.08.2023 14:02, Jan Beulich wrote:
> When !MEM_SHARING no useful output is produced. Move the function into
> mm/mem_sharing.c while conditionalizing the call to it, thus allowing to
> drop it altogether from Arm (and eliminating the need to introduce stubs
> on PPC and RISC-V).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Tamas - any chance of an ack?

Thanks, Jan

> ---
> I wasn't really sure whether introducing a stub in xen/mm.h would be any
> better than adding the (further) #ifdef to dump_domains().
> 
> We could go further and also eliminate the need for the stub variants
> of mem_sharing_get_nr_{shared,saved}_mfns() by moving the
> XENMEM_get_sharing_{shared,freed}_pages cases in
> {,compat_}arch_memory_op() into the already existing #ifdef-s there.
> Returning an error for those sub-ops may be slightly more appropriate
> than returning 0 when !MEM_SHARING.
> 
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1297,10 +1297,6 @@ void free_init_memory(void)
>      printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
>  }
>  
> -void arch_dump_shared_mem_info(void)
> -{
> -}
> -
>  int steal_page(
>      struct domain *d, struct page_info *page, unsigned int memflags)
>  {
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -6265,13 +6265,6 @@ void memguard_unguard_stack(void *p)
>      map_pages_to_xen((unsigned long)p, virt_to_mfn(p), 1, PAGE_HYPERVISOR_RW);
>  }
>  
> -void arch_dump_shared_mem_info(void)
> -{
> -    printk("Shared frames %u -- Saved frames %u\n",
> -            mem_sharing_get_nr_shared_mfns(),
> -            mem_sharing_get_nr_saved_mfns());
> -}
> -
>  const struct platform_bad_page *__init get_platform_badpages(unsigned int *array_size)
>  {
>      u32 igd_id;
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -2329,3 +2329,10 @@ int mem_sharing_domctl(struct domain *d,
>  
>      return rc;
>  }
> +
> +void arch_dump_shared_mem_info(void)
> +{
> +    printk("Shared frames %u -- Saved frames %u\n",
> +            mem_sharing_get_nr_shared_mfns(),
> +            mem_sharing_get_nr_saved_mfns());
> +}
> --- a/xen/common/keyhandler.c
> +++ b/xen/common/keyhandler.c
> @@ -365,7 +365,9 @@ static void cf_check dump_domains(unsign
>          }
>      }
>  
> +#ifdef CONFIG_MEM_SHARING
>      arch_dump_shared_mem_info();
> +#endif
>  
>      rcu_read_unlock(&domlist_read_lock);
>  }
>

Re: [PATCH] mem-sharing: move (x86) / drop (Arm) arch_dump_shared_mem_info()

Posted by Luca Fancellu 8 months, 4 weeks ago


> On 8 Aug 2023, at 13:02, Jan Beulich <jbeulich@suse.com> wrote:
> 
> When !MEM_SHARING no useful output is produced. Move the function into
> mm/mem_sharing.c while conditionalizing the call to it, thus allowing to
> drop it altogether from Arm (and eliminating the need to introduce stubs
> on PPC and RISC-V).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> I wasn't really sure whether introducing a stub in xen/mm.h would be any
> better than adding the (further) #ifdef to dump_domains().
> 
> We could go further and also eliminate the need for the stub variants
> of mem_sharing_get_nr_{shared,saved}_mfns() by moving the
> XENMEM_get_sharing_{shared,freed}_pages cases in
> {,compat_}arch_memory_op() into the already existing #ifdef-s there.
> Returning an error for those sub-ops may be slightly more appropriate
> than returning 0 when !MEM_SHARING.
> 
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1297,10 +1297,6 @@ void free_init_memory(void)
>     printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
> }
> 
> -void arch_dump_shared_mem_info(void)
> -{
> -}
> -

Hi Jan,

For the arm part:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> #arm

Re: [PATCH] mem-sharing: move (x86) / drop (Arm) arch_dump_shared_mem_info()

Posted by Stefano Stabellini 8 months, 4 weeks ago

On Tue, 8 Aug 2023, Luca Fancellu wrote:
> > On 8 Aug 2023, at 13:02, Jan Beulich <jbeulich@suse.com> wrote:
> > 
> > When !MEM_SHARING no useful output is produced. Move the function into
> > mm/mem_sharing.c while conditionalizing the call to it, thus allowing to
> > drop it altogether from Arm (and eliminating the need to introduce stubs
> > on PPC and RISC-V).
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > ---
> > I wasn't really sure whether introducing a stub in xen/mm.h would be any
> > better than adding the (further) #ifdef to dump_domains().
> > 
> > We could go further and also eliminate the need for the stub variants
> > of mem_sharing_get_nr_{shared,saved}_mfns() by moving the
> > XENMEM_get_sharing_{shared,freed}_pages cases in
> > {,compat_}arch_memory_op() into the already existing #ifdef-s there.
> > Returning an error for those sub-ops may be slightly more appropriate
> > than returning 0 when !MEM_SHARING.
> > 
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -1297,10 +1297,6 @@ void free_init_memory(void)
> >     printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
> > }
> > 
> > -void arch_dump_shared_mem_info(void)
> > -{
> > -}
> > -
> 
> Hi Jan,
> 
> For the arm part:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> #arm

Acked-by: Stefano Stabellini <sstabellini@kernel.org>