[PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

[PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

'remove' callbacks get called whenever a device is unbound from
the driver, which can get triggered from user space.

Putting it into the __exit section means that the function gets
dropped in for built-in drivers, as pointed out by this build
warning:

`tpda_remove' referenced in section `.data' of drivers/hwtracing/coresight/coresight-tpda.o: defined in discarded section `.exit.text' of drivers/hwtracing/coresight/coresight-tpda.o
`tpdm_remove' referenced in section `.data' of drivers/hwtracing/coresight/coresight-tpdm.o: defined in discarded section `.exit.text' of drivers/hwtracing/coresight/coresight-tpdm.o

Fixes: 5b7916625c01 ("Coresight: Add TPDA link driver")
Fixes: b3c71626a933 ("Coresight: Add coresight TPDM source driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/hwtracing/coresight/coresight-tpda.c | 2 +-
 drivers/hwtracing/coresight/coresight-tpdm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 19c25c9f6157..382d648529e7 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -174,7 +174,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id)
 	return 0;
 }
 
-static void __exit tpda_remove(struct amba_device *adev)
+static void tpda_remove(struct amba_device *adev)
 {
 	struct tpda_drvdata *drvdata = dev_get_drvdata(&adev->dev);
 
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 349a82bb3270..9479a5e8c672 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -223,7 +223,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
 	return 0;
 }
 
-static void __exit tpdm_remove(struct amba_device *adev)
+static void tpdm_remove(struct amba_device *adev)
 {
 	struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
 
-- 
2.39.0

Re: [PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

[Suzuki K Poulose]

Hi Arnd,

On 26/01/2023 16:35, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 'remove' callbacks get called whenever a device is unbound from
> the driver, which can get triggered from user space.
> 
> Putting it into the __exit section means that the function gets
> dropped in for built-in drivers, as pointed out by this build
> warning:

> 
> `tpda_remove' referenced in section `.data' of drivers/hwtracing/coresight/coresight-tpda.o: defined in discarded section `.exit.text' of drivers/hwtracing/coresight/coresight-tpda.o
> `tpdm_remove' referenced in section `.data' of drivers/hwtracing/coresight/coresight-tpdm.o: defined in discarded section `.exit.text' of drivers/hwtracing/coresight/coresight-tpdm.o
> 

Thanks for the fix, I will queue this. Btw, I did try to
reproduce it locally, but couldn't trigger the warnings,
even with

CONFIG_WERROR=y

and all CORESIGHT configs builtin. I see other drivers doing the
same outside coresight too. Just curious to know why is this
any different. Is it specific to "bus" driver (e.g. AMBA) ?

Suzuki


> Fixes: 5b7916625c01 ("Coresight: Add TPDA link driver")
> Fixes: b3c71626a933 ("Coresight: Add coresight TPDM source driver")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


> ---
>   drivers/hwtracing/coresight/coresight-tpda.c | 2 +-
>   drivers/hwtracing/coresight/coresight-tpdm.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index 19c25c9f6157..382d648529e7 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -174,7 +174,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id)
>   	return 0;
>   }
>   
> -static void __exit tpda_remove(struct amba_device *adev)
> +static void tpda_remove(struct amba_device *adev)
>   {
>   	struct tpda_drvdata *drvdata = dev_get_drvdata(&adev->dev);
>   
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 349a82bb3270..9479a5e8c672 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -223,7 +223,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
>   	return 0;
>   }
>   
> -static void __exit tpdm_remove(struct amba_device *adev)
> +static void tpdm_remove(struct amba_device *adev)
>   {
>   	struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
>   

Re: [PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

[Arnd Bergmann]

On Thu, Jan 26, 2023, at 19:02, Suzuki K Poulose wrote:
> On 26/01/2023 16:35, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
> Thanks for the fix, I will queue this. Btw, I did try to
> reproduce it locally, but couldn't trigger the warnings,
> even with
>
> CONFIG_WERROR=y
>
> and all CORESIGHT configs builtin. I see other drivers doing the
> same outside coresight too. Just curious to know why is this
> any different. Is it specific to "bus" driver (e.g. AMBA) ?

The warning comes from postprocessing the object file, it's got
nothing to do with the bus type, only with a symbol in .data
referencing a symbol in .init.text. Maybe there are some
config options that keep the section from getting discarded?
Or possibly you only built the files in this directory, but did
not get to the final link?

      Arnd

Re: [PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

[Suzuki K Poulose]

On 26/01/2023 20:37, Arnd Bergmann wrote:
> On Thu, Jan 26, 2023, at 19:02, Suzuki K Poulose wrote:
>> On 26/01/2023 16:35, Arnd Bergmann wrote:
>>> From: Arnd Bergmann <arnd@arndb.de>
>> Thanks for the fix, I will queue this. Btw, I did try to
>> reproduce it locally, but couldn't trigger the warnings,
>> even with
>>
>> CONFIG_WERROR=y
>>
>> and all CORESIGHT configs builtin. I see other drivers doing the
>> same outside coresight too. Just curious to know why is this
>> any different. Is it specific to "bus" driver (e.g. AMBA) ?
> 
> The warning comes from postprocessing the object file, it's got
> nothing to do with the bus type, only with a symbol in .data
> referencing a symbol in .init.text. Maybe there are some
> config options that keep the section from getting discarded?
> Or possibly you only built the files in this directory, but did
> not get to the final link?

I did a full kernel build. Also, I see a similar issue with the 
coresight-etm4x (by code inspection) driver. Did you not hit that ?

May be there is a config option that is masking it on my end. But
the case of etm4x driver is puzzling.

$ git grep etm4_remove_amba 
drivers/hwtracing/coresight/coresight-etm4x-core.c
drivers/hwtracing/coresight/coresight-etm4x-core.c:static void __exit 
etm4_remove_amba(struct amba_device *adev)
drivers/hwtracing/coresight/coresight-etm4x-core.c:     .remove 
= etm4_remove_amba,

Suzuki


> 


>        Arnd

Re: [PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

[Arnd Bergmann]

On Fri, Jan 27, 2023, at 17:46, Suzuki K Poulose wrote:
> On 26/01/2023 20:37, Arnd Bergmann wrote:
>> On Thu, Jan 26, 2023, at 19:02, Suzuki K Poulose wrote:
>>> On 26/01/2023 16:35, Arnd Bergmann wrote:
>>>> From: Arnd Bergmann <arnd@arndb.de>
>>> Thanks for the fix, I will queue this. Btw, I did try to
>>> reproduce it locally, but couldn't trigger the warnings,
>>> even with
>>>
>>> CONFIG_WERROR=y
>>>
>>> and all CORESIGHT configs builtin. I see other drivers doing the
>>> same outside coresight too. Just curious to know why is this
>>> any different. Is it specific to "bus" driver (e.g. AMBA) ?
>> 
>> The warning comes from postprocessing the object file, it's got
>> nothing to do with the bus type, only with a symbol in .data
>> referencing a symbol in .init.text. Maybe there are some
>> config options that keep the section from getting discarded?
>> Or possibly you only built the files in this directory, but did
>> not get to the final link?
>
> I did a full kernel build. Also, I see a similar issue with the 
> coresight-etm4x (by code inspection) driver. Did you not hit that ?
>
> May be there is a config option that is masking it on my end. But
> the case of etm4x driver is puzzling.
>
> $ git grep etm4_remove_amba 
> drivers/hwtracing/coresight/coresight-etm4x-core.c
> drivers/hwtracing/coresight/coresight-etm4x-core.c:static void __exit 
> etm4_remove_amba(struct amba_device *adev)
> drivers/hwtracing/coresight/coresight-etm4x-core.c:     .remove 
> = etm4_remove_amba,

Indeed, that one clearly has the same but, but I have never
observed a warning for it.

I checked one more thing and found that I only get the warning
for 32-bit Arm builds, but not arm64. Since the etm4x driver
'depends on ARM64' for its use of asm/sysreg.h,
I never test-built it on 32-bit arm.

From the git history of arch/arm64/kernel/vmlinux.lds.S,
I can see that arm64 never discards the .exit section, see
commit 07c802bd7c39 ("arm64: vmlinux.lds.S: don't discard
.exit.* sections at link-time").

     Arnd

Re: [PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

[Suzuki K Poulose]

On 27/01/2023 17:00, Arnd Bergmann wrote:
> On Fri, Jan 27, 2023, at 17:46, Suzuki K Poulose wrote:
>> On 26/01/2023 20:37, Arnd Bergmann wrote:
>>> On Thu, Jan 26, 2023, at 19:02, Suzuki K Poulose wrote:
>>>> On 26/01/2023 16:35, Arnd Bergmann wrote:
>>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>> Thanks for the fix, I will queue this. Btw, I did try to
>>>> reproduce it locally, but couldn't trigger the warnings,
>>>> even with
>>>>
>>>> CONFIG_WERROR=y
>>>>
>>>> and all CORESIGHT configs builtin. I see other drivers doing the
>>>> same outside coresight too. Just curious to know why is this
>>>> any different. Is it specific to "bus" driver (e.g. AMBA) ?
>>>
>>> The warning comes from postprocessing the object file, it's got
>>> nothing to do with the bus type, only with a symbol in .data
>>> referencing a symbol in .init.text. Maybe there are some
>>> config options that keep the section from getting discarded?
>>> Or possibly you only built the files in this directory, but did
>>> not get to the final link?
>>
>> I did a full kernel build. Also, I see a similar issue with the
>> coresight-etm4x (by code inspection) driver. Did you not hit that ?
>>
>> May be there is a config option that is masking it on my end. But
>> the case of etm4x driver is puzzling.
>>
>> $ git grep etm4_remove_amba
>> drivers/hwtracing/coresight/coresight-etm4x-core.c
>> drivers/hwtracing/coresight/coresight-etm4x-core.c:static void __exit
>> etm4_remove_amba(struct amba_device *adev)
>> drivers/hwtracing/coresight/coresight-etm4x-core.c:     .remove
>> = etm4_remove_amba,
> 
> Indeed, that one clearly has the same but, but I have never
> observed a warning for it.
> 
> I checked one more thing and found that I only get the warning
> for 32-bit Arm builds, but not arm64. Since the etm4x driver
> 'depends on ARM64' for its use of asm/sysreg.h,
> I never test-built it on 32-bit arm.
> 
>  From the git history of arch/arm64/kernel/vmlinux.lds.S,
> I can see that arm64 never discards the .exit section, see
> commit 07c802bd7c39 ("arm64: vmlinux.lds.S: don't discard
> .exit.* sections at link-time").

That makes sense, thanks for getting to the bottom of this. I
have pushed it to coresight next.

https://git.kernel.org/coresight/c/0c1ccc158bbc

Kind regards
Suzuki



> 
>       Arnd