| Message ID | 20180919182247.1101-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v2] cpufreq: qcom-kryo: Fix section mismatch warning | expand |
On 19-09-18, 11:22, Nathan Chancellor wrote: > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from > the function qcom_cpufreq_kryo_probe() to the function > .init.text:qcom_cpufreq_kryo_get_msm_id() > The function qcom_cpufreq_kryo_probe() references > the function __init qcom_cpufreq_kryo_get_msm_id(). > This is often because qcom_cpufreq_kryo_probe lacks a __init > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong. > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is > no more mismatch warning. > > Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver") > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > > v1 -> v2: > > Add '__init' to qcom_cpufreq_kryo_probe instead of removing it from > qcom_cpufreq_kryo_get_msm_id. > > drivers/cpufreq/qcom-cpufreq-kryo.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c > index a1830fa25fc5..e001ef8ea7de 100644 > --- a/drivers/cpufreq/qcom-cpufreq-kryo.c > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c > @@ -73,7 +73,7 @@ static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void) > return version; > } > > -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) > +static int __init qcom_cpufreq_kryo_probe(struct platform_device *pdev) > { > struct opp_table *opp_tables[NR_CPUS] = {0}; > enum _msm8996_version msm8996_version; Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from > the function qcom_cpufreq_kryo_probe() to the function > .init.text:qcom_cpufreq_kryo_get_msm_id() > The function qcom_cpufreq_kryo_probe() references > the function __init qcom_cpufreq_kryo_get_msm_id(). > This is often because qcom_cpufreq_kryo_probe lacks a __init > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong. > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is > no more mismatch warning. I wonder why this driver has an exit function marked __init rather than __exit? Does that mean it gets cleaned up after kernel init, and so on unloading of the driver, the kernel jumps to unmapped memory? Does this patch now produce a warning for `qcom_cpufreq_kryo_driver` referencing `qcom_cpufreq_kryo_probe`? > > Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver") > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > > v1 -> v2: > > Add '__init' to qcom_cpufreq_kryo_probe instead of removing it from > qcom_cpufreq_kryo_get_msm_id. > > drivers/cpufreq/qcom-cpufreq-kryo.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c > index a1830fa25fc5..e001ef8ea7de 100644 > --- a/drivers/cpufreq/qcom-cpufreq-kryo.c > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c > @@ -73,7 +73,7 @@ static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void) > return version; > } > > -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) > +static int __init qcom_cpufreq_kryo_probe(struct platform_device *pdev) > { > struct opp_table *opp_tables[NR_CPUS] = {0}; > enum _msm8996_version msm8996_version; > -- > 2.19.0 > -- Thanks, ~Nick Desaulniers
On 19-09-18, 14:45, Nick Desaulniers wrote: > On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from > > the function qcom_cpufreq_kryo_probe() to the function > > .init.text:qcom_cpufreq_kryo_get_msm_id() > > The function qcom_cpufreq_kryo_probe() references > > the function __init qcom_cpufreq_kryo_get_msm_id(). > > This is often because qcom_cpufreq_kryo_probe lacks a __init > > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong. > > > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is > > no more mismatch warning. > > I wonder why this driver has an exit function marked __init rather > than __exit? I think it was just a mistake. > Does that mean it gets cleaned up after kernel init, and > so on unloading of the driver, the kernel jumps to unmapped memory? The __init/exit sections are only useful when the driver is builtin and so there is no unloading. Yeah, if you would have tried to call shutdown for the kernel, it may have crashed or something. I don't know. > Does this patch now produce a warning for `qcom_cpufreq_kryo_driver` > referencing `qcom_cpufreq_kryo_probe`? Why should it ? It doesn't though.
On 19-09-18, 14:50, Viresh Kumar wrote: > On 19-09-18, 14:45, Nick Desaulniers wrote: > > On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor > > <natechancellor@gmail.com> wrote: > > > > > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from > > > the function qcom_cpufreq_kryo_probe() to the function > > > .init.text:qcom_cpufreq_kryo_get_msm_id() > > > The function qcom_cpufreq_kryo_probe() references > > > the function __init qcom_cpufreq_kryo_get_msm_id(). > > > This is often because qcom_cpufreq_kryo_probe lacks a __init > > > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong. > > > > > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is > > > no more mismatch warning. > > > > I wonder why this driver has an exit function marked __init rather > > than __exit? > > I think it was just a mistake. > > > Does that mean it gets cleaned up after kernel init, and > > so on unloading of the driver, the kernel jumps to unmapped memory? > > The __init/exit sections are only useful when the driver is builtin > and so there is no unloading. Yeah, if you would have tried to call > shutdown for the kernel, it may have crashed or something. I don't > know. > > > Does this patch now produce a warning for `qcom_cpufreq_kryo_driver` > > referencing `qcom_cpufreq_kryo_probe`? > > Why should it ? It doesn't though. I thought you replied to my commit where I marked the exit routine with __exit and realised just now that it wasn't the case. I haven't build-tested this thing, but the question still stands. Why should it ?
On Wed, Sep 19, 2018 at 02:45:15PM -0700, Nick Desaulniers wrote: > On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from > > the function qcom_cpufreq_kryo_probe() to the function > > .init.text:qcom_cpufreq_kryo_get_msm_id() > > The function qcom_cpufreq_kryo_probe() references > > the function __init qcom_cpufreq_kryo_get_msm_id(). > > This is often because qcom_cpufreq_kryo_probe lacks a __init > > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong. > > > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is > > no more mismatch warning. > > I wonder why this driver has an exit function marked __init rather > than __exit? Does that mean it gets cleaned up after kernel init, and > so on unloading of the driver, the kernel jumps to unmapped memory? > > Does this patch now produce a warning for `qcom_cpufreq_kryo_driver` > referencing `qcom_cpufreq_kryo_probe`? > Hmmm I could have sworn I built this before sending it, although my config might have been messed up but yes, I do see that warning. WARNING: vmlinux.o(.data+0x117618): Section mismatch in reference from the variable qcom_cpufreq_kryo_driver to the function .init.text:qcom_cpufreq_kryo_probe() The variable qcom_cpufreq_kryo_driver references the function __init qcom_cpufreq_kryo_probe() I suppose v1 might be the better option then as with the other patch, marking qcom_cpufreq_kryo_driver doesn't seem correct. I'm open to suggestions though. Nathan > > > > Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver") > > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > --- > > > > v1 -> v2: > > > > Add '__init' to qcom_cpufreq_kryo_probe instead of removing it from > > qcom_cpufreq_kryo_get_msm_id. > > > > drivers/cpufreq/qcom-cpufreq-kryo.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c > > index a1830fa25fc5..e001ef8ea7de 100644 > > --- a/drivers/cpufreq/qcom-cpufreq-kryo.c > > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c > > @@ -73,7 +73,7 @@ static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void) > > return version; > > } > > > > -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) > > +static int __init qcom_cpufreq_kryo_probe(struct platform_device *pdev) > > { > > struct opp_table *opp_tables[NR_CPUS] = {0}; > > enum _msm8996_version msm8996_version; > > -- > > 2.19.0 > > > > > -- > Thanks, > ~Nick Desaulniers
On Wed, Sep 19, 2018 at 11:54 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 19-09-18, 14:50, Viresh Kumar wrote: > > On 19-09-18, 14:45, Nick Desaulniers wrote: > > > On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor > > > <natechancellor@gmail.com> wrote: > > > > > > > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from > > > > the function qcom_cpufreq_kryo_probe() to the function > > > > .init.text:qcom_cpufreq_kryo_get_msm_id() > > > > The function qcom_cpufreq_kryo_probe() references > > > > the function __init qcom_cpufreq_kryo_get_msm_id(). > > > > This is often because qcom_cpufreq_kryo_probe lacks a __init > > > > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong. > > > > > > > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is > > > > no more mismatch warning. > > > > > > I wonder why this driver has an exit function marked __init rather > > > than __exit? > > > > I think it was just a mistake. > > > > > Does that mean it gets cleaned up after kernel init, and > > > so on unloading of the driver, the kernel jumps to unmapped memory? > > > > The __init/exit sections are only useful when the driver is builtin > > and so there is no unloading. Yeah, if you would have tried to call > > shutdown for the kernel, it may have crashed or something. I don't > > know. > > > > > Does this patch now produce a warning for `qcom_cpufreq_kryo_driver` > > > referencing `qcom_cpufreq_kryo_probe`? > > > > Why should it ? It doesn't though. > > I thought you replied to my commit where I marked the exit routine > with __exit and realised just now that it wasn't the case. I haven't > build-tested this thing, but the question still stands. Why should it > ? Because __init things go away at one point and calling them from the other sections is a bad idea. OTOH, __exit things are simply not needed in built-in drivers and they are never there if the driver is built-in, so calling them from the other sections is a bad idea too. Can you guys, please, prepare *one* patch fixing all of the __init/__exit annotations in this driver and post it? Thanks, Rafael
On Thu, Sep 20, 2018 at 12:45:55AM +0200, Rafael J. Wysocki wrote: > On Wed, Sep 19, 2018 at 11:54 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 19-09-18, 14:50, Viresh Kumar wrote: > > > On 19-09-18, 14:45, Nick Desaulniers wrote: > > > > On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor > > > > <natechancellor@gmail.com> wrote: > > > > > > > > > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from > > > > > the function qcom_cpufreq_kryo_probe() to the function > > > > > .init.text:qcom_cpufreq_kryo_get_msm_id() > > > > > The function qcom_cpufreq_kryo_probe() references > > > > > the function __init qcom_cpufreq_kryo_get_msm_id(). > > > > > This is often because qcom_cpufreq_kryo_probe lacks a __init > > > > > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong. > > > > > > > > > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is > > > > > no more mismatch warning. > > > > > > > > I wonder why this driver has an exit function marked __init rather > > > > than __exit? > > > > > > I think it was just a mistake. > > > > > > > Does that mean it gets cleaned up after kernel init, and > > > > so on unloading of the driver, the kernel jumps to unmapped memory? > > > > > > The __init/exit sections are only useful when the driver is builtin > > > and so there is no unloading. Yeah, if you would have tried to call > > > shutdown for the kernel, it may have crashed or something. I don't > > > know. > > > > > > > Does this patch now produce a warning for `qcom_cpufreq_kryo_driver` > > > > referencing `qcom_cpufreq_kryo_probe`? > > > > > > Why should it ? It doesn't though. > > > > I thought you replied to my commit where I marked the exit routine > > with __exit and realised just now that it wasn't the case. I haven't > > build-tested this thing, but the question still stands. Why should it > > ? > > Because __init things go away at one point and calling them from the > other sections is a bad idea. OTOH, __exit things are simply not > needed in built-in drivers and they are never there if the driver is > built-in, so calling them from the other sections is a bad idea too. > > Can you guys, please, prepare *one* patch fixing all of the > __init/__exit annotations in this driver and post it? > > Thanks, > Rafael Hi Rafael, Yes, I will work on this and send out a v3 for review. Thanks, Nathan
On 20-09-18, 00:45, Rafael J. Wysocki wrote: > Because __init things go away at one point and calling them from the > other sections is a bad idea. OTOH, __exit things are simply not > needed in built-in drivers and they are never there if the driver is > built-in, so calling them from the other sections is a bad idea too. I assumed that the driver is calling platform_driver_probe() and so probe() can be marked __init. But it doesn't do that to make deferred probing work.
diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c index a1830fa25fc5..e001ef8ea7de 100644 --- a/drivers/cpufreq/qcom-cpufreq-kryo.c +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c @@ -73,7 +73,7 @@ static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void) return version; } -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) +static int __init qcom_cpufreq_kryo_probe(struct platform_device *pdev) { struct opp_table *opp_tables[NR_CPUS] = {0}; enum _msm8996_version msm8996_version;
WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from the function qcom_cpufreq_kryo_probe() to the function .init.text:qcom_cpufreq_kryo_get_msm_id() The function qcom_cpufreq_kryo_probe() references the function __init qcom_cpufreq_kryo_get_msm_id(). This is often because qcom_cpufreq_kryo_probe lacks a __init annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong. Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is no more mismatch warning. Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver") Reported-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- v1 -> v2: Add '__init' to qcom_cpufreq_kryo_probe instead of removing it from qcom_cpufreq_kryo_get_msm_id. drivers/cpufreq/qcom-cpufreq-kryo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)