Re: [PATCH v6 08/15] x86/efi: create new early memory allocator

From: Daniel Kiper <daniel.kiper@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Juergen Gross <JGross@suse.com>,
	sstabellini@kernel.org, andrew.cooper3@citrix.com,
	cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com,
	david.vrabel@citrix.com, xen-devel@lists.xenproject.org,
	qiaowei.ren@intel.com, gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v6 08/15] x86/efi: create new early memory allocator
Date: Thu, 22 Sep 2016 14:07:08 +0200	[thread overview]
Message-ID: <20160922120708.GR28753@olila.local.net-space.pl> (raw)
In-Reply-To: <57E3DBDA020000780011194A@prv-mh.provo.novell.com>

On Thu, Sep 22, 2016 at 05:25:46AM -0600, Jan Beulich wrote:
> >>> On 22.09.16 at 12:52, <daniel.kiper@oracle.com> wrote:
> > On Wed, Sep 21, 2016 at 03:42:09AM -0600, Jan Beulich wrote:
> >> >>> On 20.09.16 at 20:45, <daniel.kiper@oracle.com> wrote:
> >> > On Tue, Sep 20, 2016 at 07:46:56AM -0600, Jan Beulich wrote:
> >> >> >>> On 20.09.16 at 12:52, <daniel.kiper@oracle.com> wrote:
> >
> > [...]
> >
> >> >> > Do you suggest that I should move out of #ifndef CONFIG_ARM all ebmalloc
> > stuff
> >> >> > except free_ebmalloc_unused_mem(). Even if it is not used on ARM right
> > now?
> >> >>
> >> >> That's not the static function, is it? The function you name should
> >> >> actually be called on ARM too (as I did point out elsewhere already),
> >> >> just to not leave a trap open for someone to fall into when (s)he
> >> >> adds a first use of the allocator on ARM.
> >> >
> >> > Well, I think that sane person doing that would analyze how ebmalloc works
> >> > on x86 and then move (align to ARM needs if required) all its machinery
> >> > (including free_ebmalloc_unused_mem()) to run on ARM. At least I would do
> >> > that. This way he/she would avoid issues mentioned by you. However, if you
> >> > still prefer your way I can do that. Though I am not sure about the rest of
> >> > ebmalloc stuff. Should I move it out of #ifndef CONFIG_ARM? Looking at your
> >> > earlier replies I see that yes. Am I correct?
> >>
> >> Yes.
> >
> > This does not work because if I build Xen for ARM then I got:
> > boot.c:589:21: error: 'ebmalloc' defined but not used
> > [-Werror=unused-function]
> >  static void __init *ebmalloc(size_t size)
>
> Quote from an earlier reply of mine:
>
>   Arguably the one static function may better be, as other
>   workarounds to avoid the "unused" compiler warning wouldn't
>   be any better.
>
> and then later
>
>   You misunderstood - I said that for this one (unused) static
>   function such an #ifdef is probably okay, as the presumably
>   smallest possible workaround.
>
> I really have no idea what else to say.

Sorry, however, sometimes it is very difficult to understand what are
you referring to. If you could be more specific then I will be happy.

Anyway, now it looks that you wish something like that:

#ifndef CONFIG_ARM
/* Whole x86 ebmalloc stuff. */
...
#else
void __init free_ebmalloc_unused_mem(void)
{
}
#endif

and then call free_ebmalloc_unused_mem() from e.g.
xen/arch/arm/setup.c:init_done(). Am I right?

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel