[Xen-devel,RFC,07/11] Add kernel helper functions

Message ID 20180102092809.1841-8-manish.jaggi@linaro.org
State New
Headers show
Series
  • acpi: arm: IORT Support for Xen
Related show

Commit Message

Manish Jaggi Jan. 2, 2018, 9:28 a.m.
From: Manish Jaggi <manish.jaggi@linaro.org>

Add kalloc kfree functions from linux kernel.

Signed-off-by: Manish Jaggi <manish.jaggi@linaro.org>
---
 xen/include/xen/kernel.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Julien Grall Jan. 18, 2018, 6:55 p.m. | #1
Hi Manish,

Please use scripts/get_maitainers.pl to CC relevant maintainers.

On 02/01/18 09:28, manish.jaggi@linaro.org wrote:
> From: Manish Jaggi <manish.jaggi@linaro.org>
> 
> Add kalloc kfree functions from linux kernel.

This does not description all the functions you added. But I am really 
not convinced this is the right place to do that.

This file is included in many places that should not use kmalloc & co 
but instead Xen version.

If you still want to add it in an header, I think it would make sense to 
create a header that will contain linuxism. So it get included only 
where it is needed.

> 
> Signed-off-by: Manish Jaggi <manish.jaggi@linaro.org>
> ---
>   xen/include/xen/kernel.h | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> index 548b64da9f..78517f6caa 100644
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -7,6 +7,16 @@
>   
>   #include <xen/types.h>
>   
> +/* Xen: Define compatibility functions */
> +#define FW_BUG         "[Firmware Bug]: "
> +#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
> +#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
> +
> +/* Alias to Xen allocation helpers */
> +#define kfree xfree
> +#define kmalloc(size, flags)            _xmalloc(size, sizeof(void *))
> +#define kzalloc(size, flags)            _xzalloc(size, sizeof(void *))
> +
>   /*
>    * min()/max() macros that also do
>    * strict type-checking.. See the
> 

Cheers,
Jan Beulich Jan. 19, 2018, 9:33 a.m. | #2
>>> On 18.01.18 at 19:55, <julien.grall@linaro.org> wrote:
> On 02/01/18 09:28, manish.jaggi@linaro.org wrote:
>> From: Manish Jaggi <manish.jaggi@linaro.org>
>> 
>> Add kalloc kfree functions from linux kernel.
> 
> This does not description all the functions you added. But I am really 
> not convinced this is the right place to do that.
> 
> This file is included in many places that should not use kmalloc & co 
> but instead Xen version.
> 
> If you still want to add it in an header, I think it would make sense to 
> create a header that will contain linuxism. So it get included only 
> where it is needed.

Indeed - if we want such helpers at all, they should be made as
invisible to any unrelated code as possible. Since Linux sources
can't normally be imported verbatim anyway, it may be best to
have such helpers near the to of affected .c files, restricted to
just the helpers that are actually needed in each individual file.
Only if this results in meaningful duplication I would consider it
reasonable to introduce a helper header.

Jan
Sameer Goel Feb. 8, 2018, 9:56 p.m. | #3
On 1/2/2018 2:28 AM, manish.jaggi@linaro.org wrote:
> From: Manish Jaggi <manish.jaggi@linaro.org>
>
> Add kalloc kfree functions from linux kernel.
>
> Signed-off-by: Manish Jaggi <manish.jaggi@linaro.org>
> ---
>  xen/include/xen/kernel.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> index 548b64da9f..78517f6caa 100644
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -7,6 +7,16 @@
>  
>  #include <xen/types.h>
>  
You might want to swap 06 and 07. Once you do that I can remove the following defines from kernel.h
> +/* Xen: Define compatibility functions */
> +#define FW_BUG         "[Firmware Bug]: "
> +#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
> +#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
> +
> +/* Alias to Xen allocation helpers */
> +#define kfree xfree
> +#define kmalloc(size, flags)            _xmalloc(size, sizeof(void *))
> +#define kzalloc(size, flags)            _xzalloc(size, sizeof(void *))
> +
>  /*
>   * min()/max() macros that also do
>   * strict type-checking.. See the

Patch

diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64da9f..78517f6caa 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -7,6 +7,16 @@ 
 
 #include <xen/types.h>
 
+/* Xen: Define compatibility functions */
+#define FW_BUG         "[Firmware Bug]: "
+#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
+#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
+
+/* Alias to Xen allocation helpers */
+#define kfree xfree
+#define kmalloc(size, flags)            _xmalloc(size, sizeof(void *))
+#define kzalloc(size, flags)            _xzalloc(size, sizeof(void *))
+
 /*
  * min()/max() macros that also do
  * strict type-checking.. See the