xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Julien Grall <julien@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	Wei Liu <wl@xen.org>
Subject: Re: [PATCH v3 01/22] mm: introduce xvmalloc() et al and use for grant table allocations
Date: Mon, 3 May 2021 16:54:19 +0200	[thread overview]
Message-ID: <YJAOm+rmKb5gbYJq@Air-de-Roger> (raw)
In-Reply-To: <aeb6aa8e-7c90-be22-1888-21b7b178e1d1@suse.com>

On Mon, May 03, 2021 at 03:50:48PM +0200, Jan Beulich wrote:
> On 03.05.2021 13:31, Roger Pau Monné wrote:
> > On Thu, Apr 22, 2021 at 04:43:39PM +0200, Jan Beulich wrote:
> >> All of the array allocations in grant_table_init() can exceed a page's
> >> worth of memory, which xmalloc()-based interfaces aren't really suitable
> >> for after boot. We also don't need any of these allocations to be
> >> physically contiguous.. Introduce interfaces dynamically switching
> >> between xmalloc() et al and vmalloc() et al, based on requested size,
> >> and use them instead.
> >>
> >> All the wrappers in the new header get cloned mostly verbatim from
> >> xmalloc.h, with the sole adjustment to switch unsigned long to size_t
> >> for sizes and to unsigned int for alignments.
> > 
> > We seem to be growing a non-trivial amount of memory allocation
> > families of functions: xmalloc, vmalloc and now xvmalloc.
> > 
> > I think from a consumer PoV it would make sense to only have two of
> > those: one for allocations that require to be physically contiguous,
> > and one for allocation that don't require it.
> > 
> > Even then, requesting for physically contiguous allocations could be
> > done by passing a flag to the same interface that's used for
> > non-contiguous allocations.
> > 
> > Maybe another option would be to expand the existing
> > v{malloc,realloc,...} set of functions to have your proposed behaviour
> > for xv{malloc,realloc,...}?
> 
> All of this and some of your remarks further down has already been
> discussed. A working group has been formed. No progress since. Yes,
> a smaller set of interfaces may be the way to go. Controlling
> behavior via flags, otoh, is very much not malloc()-like. Making
> existing functions have the intended new behavior is a no-go without
> auditing all present uses, to find those few which actually may need
> physically contiguous allocations.

But you could make your proposed xvmalloc logic the implementation
behind vmalloc, as that would still be perfectly fine and safe? (ie:
existing users of vmalloc already expect non-physically contiguous
memory). You would just optimize the size < PAGE_SIZE for that
interface?

> Having seen similar naming elsewhere, I did propose xnew() /
> xdelete() (plus array and flex-struct counterparts) as the single
> new recommended interface; didn't hear back yet. But we'd switch to
> that gradually, so intermediately there would still be a larger set
> of interfaces.
> 
> I'm not convinced we should continue to have byte-granular allocation
> functions producing physically contiguous memory. I think the page
> allocator should be used directly in such cases.
> 
> >> --- /dev/null
> >> +++ b/xen/include/xen/xvmalloc.h
> >> @@ -0,0 +1,73 @@
> >> +
> >> +#ifndef __XVMALLOC_H__
> >> +#define __XVMALLOC_H__
> >> +
> >> +#include <xen/cache.h>
> >> +#include <xen/types.h>
> >> +
> >> +/*
> >> + * Xen malloc/free-style interface for allocations possibly exceeding a page's
> >> + * worth of memory, as long as there's no need to have physically contiguous
> >> + * memory allocated.  These should be used in preference to xmalloc() et al
> >> + * whenever the size is not known to be constrained to at most a single page.
> > 
> > Even when it's know that size <= PAGE_SIZE this helpers are
> > appropriate as they would end up using xmalloc, so I think it's fine to
> > recommend them universally as long as there's no need to alloc
> > physically contiguous memory?
> > 
> > Granted there's a bit more overhead from the logic to decide between
> > using xmalloc or vmalloc &c, but that's IMO not that big of a deal in
> > order to not recommend this interface globally for non-contiguous
> > alloc.
> 
> As long as xmalloc() and vmalloc() are meant stay around as separate
> interfaces, I wouldn't want to "forbid" their use when it's sufficiently
> clear that they would be chosen by the new function anyway. Otoh, if the
> new function became more powerful in terms of falling back to the

What do you mean with more powerful here?

Thanks, Roger.


  reply	other threads:[~2021-05-03 14:54 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-22 14:38 [PATCH v3 00/22] xvmalloc() / x86 xstate area / x86 CPUID / AMX+XFD Jan Beulich
2021-04-22 14:43 ` [PATCH v3 01/22] mm: introduce xvmalloc() et al and use for grant table allocations Jan Beulich
2021-05-03 11:31   ` Roger Pau Monné
2021-05-03 13:50     ` Jan Beulich
2021-05-03 14:54       ` Roger Pau Monné [this message]
2021-05-03 15:21         ` Jan Beulich
2021-05-03 16:39           ` Roger Pau Monné
2021-04-22 14:44 ` [PATCH v3 02/22] x86/xstate: use xvzalloc() for save area allocation Jan Beulich
2021-05-05 13:29   ` Roger Pau Monné
2021-04-22 14:44 ` [PATCH v3 03/22] x86/xstate: re-size save area when CPUID policy changes Jan Beulich
2021-05-03 13:57   ` Andrew Cooper
2021-05-03 14:22     ` Jan Beulich
2021-05-11 16:41       ` Andrew Cooper
2021-05-17  7:33         ` Jan Beulich
2021-04-22 14:45 ` [PATCH v3 04/22] x86/xstate: re-use valid_xcr0() for boot-time checks Jan Beulich
2021-05-03 11:53   ` Andrew Cooper
2021-04-22 14:45 ` [PATCH v3 05/22] x86/xstate: drop xstate_offsets[] and xstate_sizes[] Jan Beulich
2021-05-03 16:10   ` Andrew Cooper
2021-05-04  7:57     ` Jan Beulich
2021-04-22 14:46 ` [PATCH v3 06/22] x86/xstate: replace xsave_cntxt_size and drop XCNTXT_MASK Jan Beulich
2021-04-22 14:47 ` [PATCH v3 07/22] x86/xstate: avoid accounting for unsupported components Jan Beulich
2021-04-22 14:47 ` [PATCH v3 08/22] x86: use xvmalloc() for extended context buffer allocations Jan Beulich
2021-04-22 14:48 ` [PATCH v3 09/22] x86/xstate: enable AMX components Jan Beulich
2021-04-22 14:50 ` [PATCH v3 10/22] x86/CPUID: adjust extended leaves out of range clearing Jan Beulich
2021-04-22 14:50 ` [PATCH v3 11/22] x86/CPUID: move bounding of max_{,sub}leaf fields to library code Jan Beulich
2021-04-22 14:51 ` [PATCH v3 12/22] x86/CPUID: enable AMX leaves Jan Beulich
2021-04-22 14:52 ` [PATCH v3 13/22] x86: XFD enabling Jan Beulich
2021-04-22 14:53 ` [PATCH v3 14/22] x86emul: introduce X86EMUL_FPU_{tilecfg,tile} Jan Beulich
2021-04-22 14:53 ` [PATCH v3 15/22] x86emul: support TILERELEASE Jan Beulich
2021-04-22 14:53 ` [PATCH v3 16/22] x86: introduce struct for TILECFG register Jan Beulich
2021-04-22 14:54 ` [PATCH v3 17/22] x86emul: support {LD,ST}TILECFG Jan Beulich
2021-04-22 14:55 ` [PATCH v3 18/22] x86emul: support TILEZERO Jan Beulich
2021-04-22 14:55 ` [PATCH v3 19/22] x86emul: support TILELOADD{,T1} and TILESTORE Jan Beulich
2021-04-22 15:06   ` Jan Beulich
2021-04-22 15:11     ` Jan Beulich
2021-04-26  7:12       ` Paul Durrant
2021-04-29  9:40         ` Jan Beulich
2021-04-22 14:56 ` [PATCH v3 20/22] x86emul: support tile multiplication insns Jan Beulich
2021-04-22 14:57 ` [PATCH v3 21/22] x86emul: test AMX insns Jan Beulich
2021-04-22 14:57 ` [PATCH v3 22/22] x86: permit guests to use AMX and XFD Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJAOm+rmKb5gbYJq@Air-de-Roger \
    --to=roger.pau@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH v3 01/22] mm: introduce xvmalloc() et al and use for grant table allocations' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).