2019-04-07 02:27:39

by Tri Vo

[permalink] [raw]
Subject: [PATCH] module: add stub for within_module

Provide a stub for within_module() when CONFIG_MODULES is not set. This
is needed to build CONFIG_GCOV_KERNEL.

Fixes: 8c3d220cb6b5 ("gcov: clang support")
Suggested-by: Matthew Wilcox <[email protected]>
Reported-by: Randy Dunlap <[email protected]>
Reported-by: kbuild test robot <[email protected]>
Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
Signed-off-by: Tri Vo <[email protected]>
---
include/linux/module.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 5bf5dcd91009..47190ebb70bf 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -709,6 +709,11 @@ static inline bool is_module_text_address(unsigned long addr)
return false;
}

+static inline bool within_module(unsigned long addr, const struct module *mod)
+{
+ return false;
+}
+
/* Get/put a kernel symbol (calls should be symmetric) */
#define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x); })
#define symbol_put(x) do { } while (0)
--
2.21.0.392.gf8f6787159e-goog


2019-04-08 18:55:25

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] module: add stub for within_module

On Sat, Apr 6, 2019 at 7:26 PM Tri Vo <[email protected]> wrote:
>
> Provide a stub for within_module() when CONFIG_MODULES is not set. This
> is needed to build CONFIG_GCOV_KERNEL.
>
> Fixes: 8c3d220cb6b5 ("gcov: clang support")

The above commit got backed out of the -mm tree, due to the issue this
patch addresses, so not sure it provides the correct context for the
patch. Maybe that line in the commit message should be dropped?

> Suggested-by: Matthew Wilcox <[email protected]>
> Reported-by: Randy Dunlap <[email protected]>
> Reported-by: kbuild test robot <[email protected]>
> Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
> Signed-off-by: Tri Vo <[email protected]>
> ---
> include/linux/module.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 5bf5dcd91009..47190ebb70bf 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -709,6 +709,11 @@ static inline bool is_module_text_address(unsigned long addr)
> return false;
> }
>
> +static inline bool within_module(unsigned long addr, const struct module *mod)
> +{
> + return false;
> +}
> +

Do folks think that similar stubs for within_module_core and
within_module_init should be added, while we're here?

It looks like kernel/trace/ftrace.c uses them, but has proper
CONFIG_MODULE guards.

> /* Get/put a kernel symbol (calls should be symmetric) */
> #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x); })
> #define symbol_put(x) do { } while (0)
> --
> 2.21.0.392.gf8f6787159e-goog
>


--
Thanks,
~Nick Desaulniers

2019-04-08 19:40:20

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] module: add stub for within_module

On Mon, Apr 8, 2019 at 11:08 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Sat, Apr 6, 2019 at 7:26 PM Tri Vo <[email protected]> wrote:
> >
> > Provide a stub for within_module() when CONFIG_MODULES is not set. This
> > is needed to build CONFIG_GCOV_KERNEL.
> >
> > Fixes: 8c3d220cb6b5 ("gcov: clang support")
>
> The above commit got backed out of the -mm tree, due to the issue this
> patch addresses, so not sure it provides the correct context for the
> patch. Maybe that line in the commit message should be dropped?

Maybe Jessica could drop that if/when applying?

>
> > Suggested-by: Matthew Wilcox <[email protected]>
> > Reported-by: Randy Dunlap <[email protected]>
> > Reported-by: kbuild test robot <[email protected]>
> > Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
> > Signed-off-by: Tri Vo <[email protected]>
> > ---
> > include/linux/module.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 5bf5dcd91009..47190ebb70bf 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -709,6 +709,11 @@ static inline bool is_module_text_address(unsigned long addr)
> > return false;
> > }
> >
> > +static inline bool within_module(unsigned long addr, const struct module *mod)
> > +{
> > + return false;
> > +}
> > +
>
> Do folks think that similar stubs for within_module_core and
> within_module_init should be added, while we're here?
>

Otherwise, if the answer to the above is no,
Reviewed-by: Nick Desaulniers <[email protected]>

--
Thanks,
~Nick Desaulniers

2019-04-15 14:24:33

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH] module: add stub for within_module

+++ Nick Desaulniers [08/04/19 11:08 -0700]:
>On Sat, Apr 6, 2019 at 7:26 PM Tri Vo <[email protected]> wrote:
>>
>> Provide a stub for within_module() when CONFIG_MODULES is not set. This
>> is needed to build CONFIG_GCOV_KERNEL.
>>
>> Fixes: 8c3d220cb6b5 ("gcov: clang support")
>
>The above commit got backed out of the -mm tree, due to the issue this
>patch addresses, so not sure it provides the correct context for the
>patch. Maybe that line in the commit message should be dropped?

Yeah, if the commit is no longer valid, then we should drop this line
and perhaps generalize the commit message more, maybe something like
"provide a stub for within_module() to prevent build errors when
!CONFIG_MODULES".

>> Suggested-by: Matthew Wilcox <[email protected]>
>> Reported-by: Randy Dunlap <[email protected]>
>> Reported-by: kbuild test robot <[email protected]>
>> Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
>> Signed-off-by: Tri Vo <[email protected]>
>> ---
>> include/linux/module.h | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 5bf5dcd91009..47190ebb70bf 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -709,6 +709,11 @@ static inline bool is_module_text_address(unsigned long addr)
>> return false;
>> }
>>
>> +static inline bool within_module(unsigned long addr, const struct module *mod)
>> +{
>> + return false;
>> +}
>> +
>
>Do folks think that similar stubs for within_module_core and
>within_module_init should be added, while we're here?
>
>It looks like kernel/trace/ftrace.c uses them, but has proper
>CONFIG_MODULE guards.

Tri, if you plan on sending a v2, could you add Nick's suggestion
above? Would probably be good to prevent future build errors if a user
omits CONFIG_MODULE guards.

Thanks,

Jessica

2019-04-15 18:34:53

by Tri Vo

[permalink] [raw]
Subject: [PATCH v2] module: add stubs for within_module functions

Provide stubs for within_module_core(), within_module_init(), and
within_module() to prevent build errors when !CONFIG_MODULES.

v2:
- Generalized commit message, as per Jessica.
- Stubs for within_module_core() and within_module_init(), as per Nick.

Suggested-by: Matthew Wilcox <[email protected]>
Reported-by: Randy Dunlap <[email protected]>
Reported-by: kbuild test robot <[email protected]>
Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
Signed-off-by: Tri Vo <[email protected]>
---
include/linux/module.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 5bf5dcd91009..35d83765bfbd 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -709,6 +709,23 @@ static inline bool is_module_text_address(unsigned long addr)
return false;
}

+static inline bool within_module_core(unsigned long addr,
+ const struct module *mod)
+{
+ return false;
+}
+
+static inline bool within_module_init(unsigned long addr,
+ const struct module *mod)
+{
+ return false;
+}
+
+static inline bool within_module(unsigned long addr, const struct module *mod)
+{
+ return false;
+}
+
/* Get/put a kernel symbol (calls should be symmetric) */
#define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x); })
#define symbol_put(x) do { } while (0)
--
2.21.0.392.gf8f6787159e-goog

2019-04-16 15:23:22

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v2] module: add stubs for within_module functions

+++ Tri Vo [15/04/19 11:18 -0700]:
>Provide stubs for within_module_core(), within_module_init(), and
>within_module() to prevent build errors when !CONFIG_MODULES.
>
>v2:
>- Generalized commit message, as per Jessica.
>- Stubs for within_module_core() and within_module_init(), as per Nick.
>
>Suggested-by: Matthew Wilcox <[email protected]>
>Reported-by: Randy Dunlap <[email protected]>
>Reported-by: kbuild test robot <[email protected]>
>Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
>Signed-off-by: Tri Vo <[email protected]>

Applied, thanks!

Jessica

2019-04-16 17:57:03

by Tri Vo

[permalink] [raw]
Subject: Re: [PATCH v2] module: add stubs for within_module functions

On Tue, Apr 16, 2019 at 8:21 AM Jessica Yu <[email protected]> wrote:
>
> +++ Tri Vo [15/04/19 11:18 -0700]:
> >Provide stubs for within_module_core(), within_module_init(), and
> >within_module() to prevent build errors when !CONFIG_MODULES.
> >
> >v2:
> >- Generalized commit message, as per Jessica.
> >- Stubs for within_module_core() and within_module_init(), as per Nick.
> >
> >Suggested-by: Matthew Wilcox <[email protected]>
> >Reported-by: Randy Dunlap <[email protected]>
> >Reported-by: kbuild test robot <[email protected]>
> >Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
> >Signed-off-by: Tri Vo <[email protected]>
>
> Applied, thanks!

Thank you!

2019-04-16 18:57:29

by Tri Vo

[permalink] [raw]
Subject: Re: [PATCH v2] module: add stubs for within_module functions

On Tue, Apr 16, 2019 at 10:55 AM Tri Vo <[email protected]> wrote:
>
> On Tue, Apr 16, 2019 at 8:21 AM Jessica Yu <[email protected]> wrote:
> >
> > +++ Tri Vo [15/04/19 11:18 -0700]:
> > >Provide stubs for within_module_core(), within_module_init(), and
> > >within_module() to prevent build errors when !CONFIG_MODULES.
> > >
> > >v2:
> > >- Generalized commit message, as per Jessica.
> > >- Stubs for within_module_core() and within_module_init(), as per Nick.
> > >
> > >Suggested-by: Matthew Wilcox <[email protected]>
> > >Reported-by: Randy Dunlap <[email protected]>
> > >Reported-by: kbuild test robot <[email protected]>
> > >Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
> > >Signed-off-by: Tri Vo <[email protected]>
> >
> > Applied, thanks!
>
> Thank you!

Andrew,
this patch fixes 8c3d220cb6b5 ("gcov: clang support"). Could you
re-apply the gcov patch? Sorry, if it's a dumb question. I'm not
familiar with how cross-tree patches are handled in Linux.

2019-04-16 21:39:12

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2] module: add stubs for within_module functions

On Tue, 16 Apr 2019 11:56:21 -0700 Tri Vo <[email protected]> wrote:

> On Tue, Apr 16, 2019 at 10:55 AM Tri Vo <[email protected]> wrote:
> >
> > On Tue, Apr 16, 2019 at 8:21 AM Jessica Yu <[email protected]> wrote:
> > >
> > > +++ Tri Vo [15/04/19 11:18 -0700]:
> > > >Provide stubs for within_module_core(), within_module_init(), and
> > > >within_module() to prevent build errors when !CONFIG_MODULES.
> > > >
> > > >v2:
> > > >- Generalized commit message, as per Jessica.
> > > >- Stubs for within_module_core() and within_module_init(), as per Nick.
> > > >
> > > >Suggested-by: Matthew Wilcox <[email protected]>
> > > >Reported-by: Randy Dunlap <[email protected]>
> > > >Reported-by: kbuild test robot <[email protected]>
> > > >Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
> > > >Signed-off-by: Tri Vo <[email protected]>
> > >
> > > Applied, thanks!
> >
> > Thank you!
>
> Andrew,
> this patch fixes 8c3d220cb6b5 ("gcov: clang support"). Could you
> re-apply the gcov patch? Sorry, if it's a dumb question. I'm not
> familiar with how cross-tree patches are handled in Linux.

hm, I wonder what Jessica applied this patch to?

Please resend a new version of "gcov: clang support".

2019-04-17 09:01:15

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v2] module: add stubs for within_module functions

+++ Andrew Morton [16/04/19 14:38 -0700]:
>On Tue, 16 Apr 2019 11:56:21 -0700 Tri Vo <[email protected]> wrote:
>
>> On Tue, Apr 16, 2019 at 10:55 AM Tri Vo <[email protected]> wrote:
>> >
>> > On Tue, Apr 16, 2019 at 8:21 AM Jessica Yu <[email protected]> wrote:
>> > >
>> > > +++ Tri Vo [15/04/19 11:18 -0700]:
>> > > >Provide stubs for within_module_core(), within_module_init(), and
>> > > >within_module() to prevent build errors when !CONFIG_MODULES.
>> > > >
>> > > >v2:
>> > > >- Generalized commit message, as per Jessica.
>> > > >- Stubs for within_module_core() and within_module_init(), as per Nick.
>> > > >
>> > > >Suggested-by: Matthew Wilcox <[email protected]>
>> > > >Reported-by: Randy Dunlap <[email protected]>
>> > > >Reported-by: kbuild test robot <[email protected]>
>> > > >Link: https://marc.info/?l=linux-mm&m=155384681109231&w=2
>> > > >Signed-off-by: Tri Vo <[email protected]>
>> > >
>> > > Applied, thanks!
>> >
>> > Thank you!
>>
>> Andrew,
>> this patch fixes 8c3d220cb6b5 ("gcov: clang support"). Could you
>> re-apply the gcov patch? Sorry, if it's a dumb question. I'm not
>> familiar with how cross-tree patches are handled in Linux.
>
>hm, I wonder what Jessica applied this patch to?

I applied the patch that supplies the missing within_module() stubs to
the modules-next branch (a link to the repo is available in
MAINTAINERS), it's landed in linux-next by now, and it should fix the
build error reported by the kbuild test bot.

Thanks,

Jessica