2019-01-31 14:03:38

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: [Xen-devel] xen/mem-reservation API and out-of-tree kernel modules

Hello,

I am working on porting an out-of-tree kernel driver to the kernel
5.0 and that driver uses functionality provided by
drivers/xen/mem-reservation.c
module.  Since commit [1] it is not possible to build a kernel module
which uses mem-reservation API as xen_scrub_pages variable, which is
checked in
xenmem_reservation_scrub_page, became a kernel module parameter and is
now only
accessible for built-in modules:

static inline void xenmem_reservation_scrub_page(struct page *page)
^^^^^^^^^^^^^
{
    if (xen_scrub_pages)
        ^^^^^^^^^^^^^^^
        clear_highpage(page);
}

This results in link-time warning:

    WARNING: "xen_scrub_pages" [yourmodule.ko] undefined!

and thus not allowing the module to run. At the moment I can only see a
possible fix
for this by making the following change:

diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c
index 3782cf070338..85fecfec50e1 100644
--- a/drivers/xen/mem-reservation.c
+++ b/drivers/xen/mem-reservation.c
@@ -18,6 +18,7 @@

 bool __read_mostly xen_scrub_pages =
IS_ENABLED(CONFIG_XEN_SCRUB_PAGES_DEFAULT);
 core_param(xen_scrub_pages, xen_scrub_pages, bool, 0);
+EXPORT_SYMBOL(xen_scrub_pages);

but this looks a bit unusual for the kernel?

I am looking for community advice here and help

Thank you,
Oleksandr

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=197ecb3802c04499d8ff4f8cb28f6efa008067db


2019-01-31 22:52:13

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [Xen-devel] xen/mem-reservation API and out-of-tree kernel modules

On Thu, 31 Jan 2019, Oleksandr Andrushchenko wrote:
> Hello,
>
> I am working on porting an out-of-tree kernel driver to the kernel
> 5.0 and that driver uses functionality provided by
> drivers/xen/mem-reservation.c
> module.  Since commit [1] it is not possible to build a kernel module
> which uses mem-reservation API as xen_scrub_pages variable, which is
> checked in
> xenmem_reservation_scrub_page, became a kernel module parameter and is
> now only
> accessible for built-in modules:
>
> static inline void xenmem_reservation_scrub_page(struct page *page)
> ^^^^^^^^^^^^^
> {
>     if (xen_scrub_pages)
>         ^^^^^^^^^^^^^^^
>         clear_highpage(page);
> }
>
> This results in link-time warning:
>
>     WARNING: "xen_scrub_pages" [yourmodule.ko] undefined!
>
> and thus not allowing the module to run. At the moment I can only see a
> possible fix
> for this by making the following change:
>
> diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c
> index 3782cf070338..85fecfec50e1 100644
> --- a/drivers/xen/mem-reservation.c
> +++ b/drivers/xen/mem-reservation.c
> @@ -18,6 +18,7 @@
>
>  bool __read_mostly xen_scrub_pages =
> IS_ENABLED(CONFIG_XEN_SCRUB_PAGES_DEFAULT);
>  core_param(xen_scrub_pages, xen_scrub_pages, bool, 0);
> +EXPORT_SYMBOL(xen_scrub_pages);
>
> but this looks a bit unusual for the kernel?
>
> I am looking for community advice here and help
>
> Thank you,
> Oleksandr
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=197ecb3802c04499d8ff4f8cb28f6efa008067db

The alternative would be to turn xenmem_reservation_scrub_page into a
regular function (not a static inline)?

2019-02-01 08:28:56

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [Xen-devel] xen/mem-reservation API and out-of-tree kernel modules

On Thu, Jan 31, 2019 at 01:44:15PM -0800, Stefano Stabellini wrote:
> The alternative would be to turn xenmem_reservation_scrub_page into a
> regular function (not a static inline)?

All that is a moot point until said currently out of tree module gets
submitted for inclusion anyway.

2019-02-01 08:50:33

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: Re: [Xen-devel] xen/mem-reservation API and out-of-tree kernel modules

On 2/1/19 10:27 AM, Christoph Hellwig wrote:
> On Thu, Jan 31, 2019 at 01:44:15PM -0800, Stefano Stabellini wrote:
>> The alternative would be to turn xenmem_reservation_scrub_page into a
>> regular function (not a static inline)?
> All that is a moot point until said currently out of tree module gets
> submitted for inclusion anyway.
Indeed this is a moot point, so I can't argue here.
But this is how it is and unfortunately we have to live
with those modules and depend on 3rd parties willing or not
to disclose their sources to public...

2019-02-01 08:50:33

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: Re: [Xen-devel] xen/mem-reservation API and out-of-tree kernel modules

On 1/31/19 11:44 PM, Stefano Stabellini wrote:
> On Thu, 31 Jan 2019, Oleksandr Andrushchenko wrote:
>> Hello,
>>
>> I am working on porting an out-of-tree kernel driver to the kernel
>> 5.0 and that driver uses functionality provided by
>> drivers/xen/mem-reservation.c
>> module.  Since commit [1] it is not possible to build a kernel module
>> which uses mem-reservation API as xen_scrub_pages variable, which is
>> checked in
>> xenmem_reservation_scrub_page, became a kernel module parameter and is
>> now only
>> accessible for built-in modules:
>>
>> static inline void xenmem_reservation_scrub_page(struct page *page)
>> ^^^^^^^^^^^^^
>> {
>>     if (xen_scrub_pages)
>>         ^^^^^^^^^^^^^^^
>>         clear_highpage(page);
>> }
>>
>> This results in link-time warning:
>>
>>     WARNING: "xen_scrub_pages" [yourmodule.ko] undefined!
>>
>> and thus not allowing the module to run. At the moment I can only see a
>> possible fix
>> for this by making the following change:
>>
>> diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c
>> index 3782cf070338..85fecfec50e1 100644
>> --- a/drivers/xen/mem-reservation.c
>> +++ b/drivers/xen/mem-reservation.c
>> @@ -18,6 +18,7 @@
>>
>>  bool __read_mostly xen_scrub_pages =
>> IS_ENABLED(CONFIG_XEN_SCRUB_PAGES_DEFAULT);
>>  core_param(xen_scrub_pages, xen_scrub_pages, bool, 0);
>> +EXPORT_SYMBOL(xen_scrub_pages);
>>
>> but this looks a bit unusual for the kernel?
>>
>> I am looking for community advice here and help
>>
>> Thank you,
>> Oleksandr
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=197ecb3802c04499d8ff4f8cb28f6efa008067db
> The alternative would be to turn xenmem_reservation_scrub_page into a
> regular function (not a static inline)?
Yes, it seems there is no other reasonable solution to this, but
a regular function. I'll send a patch for that

Thank you,
Oleksandr

2019-02-01 08:50:37

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [Xen-devel] xen/mem-reservation API and out-of-tree kernel modules

On Fri, Feb 01, 2019 at 08:38:43AM +0000, Oleksandr Andrushchenko wrote:
> On 2/1/19 10:27 AM, Christoph Hellwig wrote:
> > On Thu, Jan 31, 2019 at 01:44:15PM -0800, Stefano Stabellini wrote:
> >> The alternative would be to turn xenmem_reservation_scrub_page into a
> >> regular function (not a static inline)?
> > All that is a moot point until said currently out of tree module gets
> > submitted for inclusion anyway.
> Indeed this is a moot point, so I can't argue here.
> But this is how it is and unfortunately we have to live
> with those modules and depend on 3rd parties willing or not
> to disclose their sources to public...

The point is that the kernel does generally not export interfaces
not used by in-tree modules. So there is no reason to change anything
here.

2019-02-01 09:15:11

by Jürgen Groß

[permalink] [raw]
Subject: Re: [Xen-devel] xen/mem-reservation API and out-of-tree kernel modules

On 01/02/2019 09:39, Oleksandr Andrushchenko wrote:
> On 1/31/19 11:44 PM, Stefano Stabellini wrote:
>> On Thu, 31 Jan 2019, Oleksandr Andrushchenko wrote:
>>> Hello,
>>>
>>> I am working on porting an out-of-tree kernel driver to the kernel
>>> 5.0 and that driver uses functionality provided by
>>> drivers/xen/mem-reservation.c
>>> module.  Since commit [1] it is not possible to build a kernel module
>>> which uses mem-reservation API as xen_scrub_pages variable, which is
>>> checked in
>>> xenmem_reservation_scrub_page, became a kernel module parameter and is
>>> now only
>>> accessible for built-in modules:
>>>
>>> static inline void xenmem_reservation_scrub_page(struct page *page)
>>> ^^^^^^^^^^^^^
>>> {
>>>     if (xen_scrub_pages)
>>>         ^^^^^^^^^^^^^^^
>>>         clear_highpage(page);
>>> }
>>>
>>> This results in link-time warning:
>>>
>>>     WARNING: "xen_scrub_pages" [yourmodule.ko] undefined!
>>>
>>> and thus not allowing the module to run. At the moment I can only see a
>>> possible fix
>>> for this by making the following change:
>>>
>>> diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c
>>> index 3782cf070338..85fecfec50e1 100644
>>> --- a/drivers/xen/mem-reservation.c
>>> +++ b/drivers/xen/mem-reservation.c
>>> @@ -18,6 +18,7 @@
>>>
>>>  bool __read_mostly xen_scrub_pages =
>>> IS_ENABLED(CONFIG_XEN_SCRUB_PAGES_DEFAULT);
>>>  core_param(xen_scrub_pages, xen_scrub_pages, bool, 0);
>>> +EXPORT_SYMBOL(xen_scrub_pages);
>>>
>>> but this looks a bit unusual for the kernel?
>>>
>>> I am looking for community advice here and help
>>>
>>> Thank you,
>>> Oleksandr
>>>
>>> [1]
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=197ecb3802c04499d8ff4f8cb28f6efa008067db
>> The alternative would be to turn xenmem_reservation_scrub_page into a
>> regular function (not a static inline)?
> Yes, it seems there is no other reasonable solution to this, but
> a regular function. I'll send a patch for that

What would you gain? This function would need to be exported.

So its either the variable or the function.


Juergen

2019-02-01 09:51:47

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: Re: [Xen-devel] xen/mem-reservation API and out-of-tree kernel modules

On 2/1/19 11:14 AM, Juergen Gross wrote:
> On 01/02/2019 09:39, Oleksandr Andrushchenko wrote:
>> On 1/31/19 11:44 PM, Stefano Stabellini wrote:
>>> On Thu, 31 Jan 2019, Oleksandr Andrushchenko wrote:
>>>> Hello,
>>>>
>>>> I am working on porting an out-of-tree kernel driver to the kernel
>>>> 5.0 and that driver uses functionality provided by
>>>> drivers/xen/mem-reservation.c
>>>> module.  Since commit [1] it is not possible to build a kernel module
>>>> which uses mem-reservation API as xen_scrub_pages variable, which is
>>>> checked in
>>>> xenmem_reservation_scrub_page, became a kernel module parameter and is
>>>> now only
>>>> accessible for built-in modules:
>>>>
>>>> static inline void xenmem_reservation_scrub_page(struct page *page)
>>>> ^^^^^^^^^^^^^
>>>> {
>>>>     if (xen_scrub_pages)
>>>>         ^^^^^^^^^^^^^^^
>>>>         clear_highpage(page);
>>>> }
>>>>
>>>> This results in link-time warning:
>>>>
>>>>     WARNING: "xen_scrub_pages" [yourmodule.ko] undefined!
>>>>
>>>> and thus not allowing the module to run. At the moment I can only see a
>>>> possible fix
>>>> for this by making the following change:
>>>>
>>>> diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c
>>>> index 3782cf070338..85fecfec50e1 100644
>>>> --- a/drivers/xen/mem-reservation.c
>>>> +++ b/drivers/xen/mem-reservation.c
>>>> @@ -18,6 +18,7 @@
>>>>
>>>>  bool __read_mostly xen_scrub_pages =
>>>> IS_ENABLED(CONFIG_XEN_SCRUB_PAGES_DEFAULT);
>>>>  core_param(xen_scrub_pages, xen_scrub_pages, bool, 0);
>>>> +EXPORT_SYMBOL(xen_scrub_pages);
>>>>
>>>> but this looks a bit unusual for the kernel?
>>>>
>>>> I am looking for community advice here and help
>>>>
>>>> Thank you,
>>>> Oleksandr
>>>>
>>>> [1]
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=197ecb3802c04499d8ff4f8cb28f6efa008067db
>>> The alternative would be to turn xenmem_reservation_scrub_page into a
>>> regular function (not a static inline)?
>> Yes, it seems there is no other reasonable solution to this, but
>> a regular function. I'll send a patch for that
> What would you gain? This function would need to be exported.
Yes, this is true, the function should be exported then
> So its either the variable or the function.
I am a bit confused with this because I'll have to export
module parameter in this case, e.g.

core_param(xen_scrub_pages, xen_scrub_pages, bool, 0);
EXPORT_SYMBOL(xen_scrub_pages);

which looks a bit unusual to me
>
> Juergen