arm: exynos_config: Restore debugfs support
diff mbox series

Message ID 20191125125515.30795-1-m.szyprowski@samsung.com
State Superseded
Headers show
Series
  • arm: exynos_config: Restore debugfs support
Related show

Commit Message

Marek Szyprowski Nov. 25, 2019, 12:55 p.m. UTC
Commit 0e4a459f56c3 ("tracing: Remove unnecessary DEBUG_FS dependency")
removed the dependency between DEBUG_FS and TRACING, so DEBUG_FS is no
longer enabled in default builds. Enable it again manually, as it provides
a lots of useful data for automated testing tools.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/configs/exynos_defconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Bartlomiej Zolnierkiewicz Nov. 25, 2019, 2:30 p.m. UTC | #1
Hi,

On 11/25/19 1:55 PM, Marek Szyprowski wrote:
> Commit 0e4a459f56c3 ("tracing: Remove unnecessary DEBUG_FS dependency")
> removed the dependency between DEBUG_FS and TRACING, so DEBUG_FS is no
> longer enabled in default builds. Enable it again manually, as it provides
> a lots of useful data for automated testing tools.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/configs/exynos_defconfig | 1 +
>  1 file changed, 1 insertion(+)

It seems that commit 0e4a459f56c3 ("tracing: Remove unnecessary DEBUG_FS
dependency") disabled DEBUG_FS also in some other ARM defconfigs.

For some of them it may be a correct change but a preferred way to
introduce such changes would be to:

- add explicit CONFIG_DEBUG_FS=y instances to all affected defconfigs
  while removing DEBUG_FS selection from TRACING config item

- let platform maintainers disable DEBUG_FS manually in corresponding
  defconfigs later if desirable

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
> index e7e4bb5ad8d5..fde84f123fbb 100644
> --- a/arch/arm/configs/exynos_defconfig
> +++ b/arch/arm/configs/exynos_defconfig
> @@ -350,6 +350,7 @@ CONFIG_PRINTK_TIME=y
>  CONFIG_DYNAMIC_DEBUG=y
>  CONFIG_DEBUG_INFO=y
>  CONFIG_MAGIC_SYSRQ=y
> +CONFIG_DEBUG_FS=y
>  CONFIG_DEBUG_KERNEL=y
>  CONFIG_SOFTLOCKUP_DETECTOR=y
>  # CONFIG_DETECT_HUNG_TASK is not set
Steven Rostedt Nov. 25, 2019, 2:39 p.m. UTC | #2
On Mon, 25 Nov 2019 15:30:39 +0100
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:

> It seems that commit 0e4a459f56c3 ("tracing: Remove unnecessary DEBUG_FS
> dependency") disabled DEBUG_FS also in some other ARM defconfigs.
> 
> For some of them it may be a correct change but a preferred way to
> introduce such changes would be to:
> 
> - add explicit CONFIG_DEBUG_FS=y instances to all affected defconfigs
>   while removing DEBUG_FS selection from TRACING config item
> 

I strongly disagree. It was wrong to assume DEBUG_FS is attached to
TRACING. If someone wanted DEBUG_FS in their def config, they should
have added it specifically. The addition of DEBUG_FS to defconfigs no
way belongs to the patch that removed DEBUG_FS from TRACING.

-- Steve


> - let platform maintainers disable DEBUG_FS manually in corresponding
>   defconfigs later if desirable
Bartlomiej Zolnierkiewicz Nov. 25, 2019, 3:31 p.m. UTC | #3
On 11/25/19 3:39 PM, Steven Rostedt wrote:
> On Mon, 25 Nov 2019 15:30:39 +0100
> Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> 
>> It seems that commit 0e4a459f56c3 ("tracing: Remove unnecessary DEBUG_FS
>> dependency") disabled DEBUG_FS also in some other ARM defconfigs.
>>
>> For some of them it may be a correct change but a preferred way to
>> introduce such changes would be to:
>>
>> - add explicit CONFIG_DEBUG_FS=y instances to all affected defconfigs
>>   while removing DEBUG_FS selection from TRACING config item
>>
> 
> I strongly disagree. It was wrong to assume DEBUG_FS is attached to
> TRACING. If someone wanted DEBUG_FS in their def config, they should
> have added it specifically. The addition of DEBUG_FS to defconfigs no

There is a theory and a practice.

In theory you are are correct. ;-)

In practice people don't manually edit configuration files nowadays.

They do 'make menuconfig' and enable what they need and disable what
they do not need.  Then they do 'make savedefconfig' and copy resulting
"stripped" defconfig file as their new platform defconfig. As a result
defconfigs rely on many default settings (also they explicitly disable
only items that are enabled by default but you don't want them).

> way belongs to the patch that removed DEBUG_FS from TRACING.
> 
> -- Steve
> 
> 
>> - let platform maintainers disable DEBUG_FS manually in corresponding
>>   defconfigs later if desirable

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Krzysztof Kozlowski Nov. 26, 2019, 1:21 a.m. UTC | #4
On Mon, 25 Nov 2019 at 23:31, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
>
> On 11/25/19 3:39 PM, Steven Rostedt wrote:
> > On Mon, 25 Nov 2019 15:30:39 +0100
> > Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> >
> >> It seems that commit 0e4a459f56c3 ("tracing: Remove unnecessary DEBUG_FS
> >> dependency") disabled DEBUG_FS also in some other ARM defconfigs.
> >>
> >> For some of them it may be a correct change but a preferred way to
> >> introduce such changes would be to:
> >>
> >> - add explicit CONFIG_DEBUG_FS=y instances to all affected defconfigs
> >>   while removing DEBUG_FS selection from TRACING config item
> >>
> >
> > I strongly disagree. It was wrong to assume DEBUG_FS is attached to
> > TRACING. If someone wanted DEBUG_FS in their def config, they should
> > have added it specifically. The addition of DEBUG_FS to defconfigs no
>
> There is a theory and a practice.
>
> In theory you are are correct. ;-)
>
> In practice people don't manually edit configuration files nowadays.
>
> They do 'make menuconfig' and enable what they need and disable what
> they do not need.  Then they do 'make savedefconfig' and copy resulting
> "stripped" defconfig file as their new platform defconfig. As a result
> defconfigs rely on many default settings (also they explicitly disable
> only items that are enabled by default but you don't want them).

I agree with Bartłomiej. Your interpretation Steven essentially
prohibits any use of savedefconfig to trim automatically the config
from unneeded options. Therefore many defconfigs which do not have
DEBUG_FS or other options directly, but they want it.

Some time ago I had patches removing specific non-existing options
from defconfigs. For each option I provided a rationale that it is
gone/etc so let's remove it from defconfig. Most of maintainers picked
them up but few (2-3?) instead run savedefconfig to clean up
everything automatically.

Best regards,
Krzysztof

Patch
diff mbox series

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index e7e4bb5ad8d5..fde84f123fbb 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -350,6 +350,7 @@  CONFIG_PRINTK_TIME=y
 CONFIG_DYNAMIC_DEBUG=y
 CONFIG_DEBUG_INFO=y
 CONFIG_MAGIC_SYSRQ=y
+CONFIG_DEBUG_FS=y
 CONFIG_DEBUG_KERNEL=y
 CONFIG_SOFTLOCKUP_DETECTOR=y
 # CONFIG_DETECT_HUNG_TASK is not set