2022-01-06 11:47:51

by Fabio Estevam

[permalink] [raw]
Subject: debugfs: Directory with parent 'regmap' already present!

Hi Mark and Matthias,

On an imx6q-sabresd board, I am still seeing:

debugfs: Directory 'dummy-iomuxc-gpr@20e0000' with parent 'regmap'
already present!

There was an earlier thread about this issue:
https://lkml.org/lkml/2021/7/26/95

but I am not sure what was the conclusion.

Any suggestions as to how to fix this warning?

Thanks,

Fabio Estevam


2022-01-06 12:07:54

by Mark Brown

[permalink] [raw]
Subject: Re: debugfs: Directory with parent 'regmap' already present!

On Thu, Jan 06, 2022 at 08:47:36AM -0300, Fabio Estevam wrote:

> On an imx6q-sabresd board, I am still seeing:

> debugfs: Directory 'dummy-iomuxc-gpr@20e0000' with parent 'regmap'
> already present!

> There was an earlier thread about this issue:
> https://lkml.org/lkml/2021/7/26/95

> but I am not sure what was the conclusion.

> Any suggestions as to how to fix this warning?

Like I said there:

| (we should probably clean up the one with no device but that's not what
| your commit does). I think what you need to look at here is that we
| store map->debugfs_name and don't overwrite it when the device is
| supplied.


Attachments:
(No filename) (629.00 B)
signature.asc (488.00 B)
Download all attachments

2022-01-06 16:57:32

by Fabio Estevam

[permalink] [raw]
Subject: Re: debugfs: Directory with parent 'regmap' already present!

Hi Mark,

On Thu, Jan 6, 2022 at 9:07 AM Mark Brown <[email protected]> wrote:

> Like I said there:
>
> | (we should probably clean up the one with no device but that's not what
> | your commit does). I think what you need to look at here is that we
> | store map->debugfs_name and don't overwrite it when the device is
> | supplied.

I added some printks to help identifying the name clash:

[ 0.274418] platform panel: Fixing up cyclic dependency with ldb
[ 0.276896] ************ 1: devname is dummy
[ 0.276926] ************ 2: name is iomuxc-gpr@20e0000
[ 0.276951] ************ 3: Forming the final name
[ 0.276979] ************ 3a: Name from map->debugfs_name
dummy-iomuxc-gpr@20e0000
[ 0.277001] ************ 4: name is dummy-iomuxc-gpr@20e0000
[ 0.277022] ************ 5: Final name is dummy-iomuxc-gpr@20e0000
[ 0.277746] No ATAGs?
[ 0.278355] hw-breakpoint: found 5 (+1 reserved) breakpoint and 1
watchpoint registers.
[ 0.278469] hw-breakpoint: maximum watchpoint size is 4 bytes.
[ 0.282007] ********** gpr succeeded
[ 0.282042] ************ 1: devname is 20e0000.pinctrl
[ 0.282070] ************ 2: name is gpr
[ 0.282093] ************ 3a: Name from map->debugfs_name
dummy-iomuxc-gpr@20e0000
[ 0.282115] ************ 4: name is dummy-iomuxc-gpr@20e0000
[ 0.282138] ************ 5: Final name is dummy-iomuxc-gpr@20e0000
[ 0.282185] debugfs: Directory 'dummy-iomuxc-gpr@20e0000' with
parent 'regmap' already present!

The duplicate "dummy-iomuxc-gpr@20e0000" comes from the lack of doing
a kfree(map->debugfs_name);

Calling 'kfree(map->debugfs_name);' like this:

diff --git a/drivers/base/regmap/regmap-debugfs.c
b/drivers/base/regmap/regmap-debugfs.c
index ad684d37c2da..18f0c7223082 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -589,6 +589,7 @@ void regmap_debugfs_init(struct regmap *map)
return;
}
name = map->debugfs_name;
+ kfree(map->debugfs_name);
} else {
name = devname;
}
@@ -600,6 +601,7 @@ void regmap_debugfs_init(struct regmap *map)
if (!map->debugfs_name)
return;
name = map->debugfs_name;
+ kfree(map->debugfs_name);
dummy_index++;
}

diff --git a/drivers/base/regmap/regmap-debugfs.c
b/drivers/base/regmap/regmap-debugfs.c
index ad684d37c2da..18f0c7223082 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -589,6 +589,7 @@ void regmap_debugfs_init(struct regmap *map)
return;
}
name = map->debugfs_name;
+ kfree(map->debugfs_name);
} else {
name = devname;
}
@@ -600,6 +601,7 @@ void regmap_debugfs_init(struct regmap *map)
if (!map->debugfs_name)
return;
name = map->debugfs_name;
+ kfree(map->debugfs_name);
dummy_index++;
}

--
2.25.1

makes the name clash disappear:

[ 0.257135] platform 2800000.ipu: Fixing up cyclic dependency with
120000.hdmi
[ 0.257460] platform 2800000.ipu: Fixing up cyclic dependency with
20e0000.iomuxc-gpr:ipu2_csi1_mux
[ 0.257782] platform 2800000.ipu: Fixing up cyclic dependency with
21dc000.mipi
[ 0.273536] platform panel: Fixing up cyclic dependency with ldb
[ 0.276013] ************ 1: devname is dummy
[ 0.276043] ************ 2: name is iomuxc-gpr@20e0000
[ 0.276067] ************ 3: Forming the final name
[ 0.276097] ************ 3a: Name from map->debugfs_name
dummy-iomuxc-gpr@20e0000
[ 0.276119] ************ 4: name is dummy-iomuxc-gpr@20e0000
[ 0.276141] ************ 5: Final name is dummy-iomuxc-gpr@20e0000
[ 0.276859] No ATAGs?
[ 0.277468] hw-breakpoint: found 5 (+1 reserved) breakpoint and 1
watchpoint registers.
[ 0.277580] hw-breakpoint: maximum watchpoint size is 4 bytes.
[ 0.281122] ********** gpr succeeded
[ 0.281156] ************ 1: devname is 20e0000.pinctrl
[ 0.281184] ************ 2: name is gpr
[ 0.281206] ************ 3a: Name from map->debugfs_name 20e0000.pinctrl
[ 0.281228] ************ 4: name is 20e0000.pinctrl
[ 0.281251] ************ 5: Final name is 20e0000.pinctrl

2022-01-06 17:25:01

by Mark Brown

[permalink] [raw]
Subject: Re: debugfs: Directory with parent 'regmap' already present!

On Thu, Jan 06, 2022 at 01:57:15PM -0300, Fabio Estevam wrote:
> On Thu, Jan 6, 2022 at 9:07 AM Mark Brown <[email protected]> wrote:

> The duplicate "dummy-iomuxc-gpr@20e0000" comes from the lack of doing
> a kfree(map->debugfs_name);
>
> Calling 'kfree(map->debugfs_name);' like this:

That doesn't sound like a good idea - if this does anything we must be
looking at the data so only adding frees will only translate using the
wrong data into a dereference of a freed pointer which seems worse.


Attachments:
(No filename) (501.00 B)
signature.asc (488.00 B)
Download all attachments