Re: [Minios-devel] [UNIKRAFT PATCH 5/8] plat/linuxu: Introduce heap size as a lib parameter

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

Hi Sharan,

the idea of the patch looks fine, but I'm not a big fan of strewing 
#ifdef CONFIG_* all over the code. What if, instead of checking the 
config parameter everywhere, there were one big #ifdef 
CONFIG_LIBUKLIBPARAM in libparam.h, which defines everything as it is 
now if it is defined, and in the #else case defines the public-facing 
macros such as UK_LIB_PARAM() as empty macros? That way, you don't need 
to #ifdef-guard all those calls inside the code.
Of course, that means the build system needs to provide the include path 
to libparam.h even if the library itself isn't compiled in, but this 
could be just done by setting the include directive in libparam's 
Makefile.uk to be CINCLUDES-y instead of 
CINCLUDES-$(CONFIG_LIBUKLIBPARAM), but maybe this unconditional addition 
to the include directories is worth less #ifdef pollution in each source 
file.
Cheers,
Florian

On 3/15/19 6:06 PM, Sharan Santhanam wrote:
> In linuxu platform, the user can overwrite the default heap size
> parameter. In this patch we introduce the heap size argument as an
> UK_LIB_PARAM which the user can modify at boot time.
>
> Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> ---
>   plat/linuxu/Makefile.uk |  2 ++
>   plat/linuxu/memory.c    | 17 ++++++++++++++++-
>   2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/plat/linuxu/Makefile.uk b/plat/linuxu/Makefile.uk
> index 2c0de76..281b5c8 100644
> --- a/plat/linuxu/Makefile.uk
> +++ b/plat/linuxu/Makefile.uk
> @@ -8,6 +8,8 @@ $(eval $(call addplat_s,linuxu,$(CONFIG_PLAT_LINUXU)))
>   ##
>   $(eval $(call addplatlib,linuxu,liblinuxuplat))
>   
> +## Adding libparam for the linuxu platform
> +$(eval $(call addplatlib_paramprefix,linuxu,liblinuxuplat,linuxu))
>   ##
>   ## Platform library definitions
>   ##
> diff --git a/plat/linuxu/memory.c b/plat/linuxu/memory.c
> index 35d0d95..6b98acb 100644
> --- a/plat/linuxu/memory.c
> +++ b/plat/linuxu/memory.c
> @@ -33,9 +33,24 @@
>    * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>    */
>   
> +#include <errno.h>
> +#include <uk/arch/types.h>
>   #include <linuxu/setup.h>
> -#include <uk/plat/memory.h>
> +#include <uk/errptr.h>
>   #include <uk/assert.h>
> +#include <linuxu/syscall.h>
> +#include <uk/plat/memory.h>
> +#ifdef CONFIG_LIBUKLIBPARAM
> +#include <uk/libparam.h>
> +#endif /* CONFIG_LIBUKLIBPARAM */
> +
> +#define MB2B           (1024 * 1024)
> +
> +static __u32 heap_size = CONFIG_LINUXU_DEFAULT_HEAPMB;
> +#ifdef CONFIG_LIBUKLIBPARAM
> +UK_LIB_PARAM(heap_size, __u32);
> +#endif /* CONFIG_LIBUKLIBPARAM */
> +
>   
>   int ukplat_memregion_count(void)
>   {
--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel