Linux 3.16: all my drivers on SPI bus report WARNING: at drivers/base/dd.c:286

Commit Message

Christophe Leroy Aug. 28, 2014, 3:53 p.m. UTC

I've been able to identify the origin of the issue. It happens since the below commit.
Do you know what should be done to fix that ?

Christophe


 From 7a40054361162d2f78f332aa868fed137beb7901 Mon Sep 17 00:00:00 2001
From: Axel Lin <[e-mail]>
Date: Sun, 30 Mar 2014 16:42:57 +0800
Subject: spi: fsl-spi: Fix memory leak

mpc8xxx_spi_probe() has set master->cleanup = mpc8xxx_spi_cleanup,
however current code overrides the setting in fsl_spi_probe() and set
master->cleanup = fsl_spi_cleanup.
Thus the memory allocated for cs is not freed anywhere.
Convert to use devm_kzalloc to fix the memory leak.

Signed-off-by: Axel Lin <[e-mail]>
Signed-off-by: Mark Brown <[e-mail]>

Comments

Stijn Devriendt Aug. 28, 2014, 8:28 p.m. UTC | #1

On Thu, Aug 28, 2014 at 5:53 PM, leroy christophe
<[e-mail]> wrote:
>
> I've been able to identify the origin of the issue. It happens since the below commit.
> Do you know what should be done to fix that ?
>
> Christophe
>
>

Actually, more things are wrong with what the driver is doing.
If inside spi_add_device() the call to device_add() fails, then that
code bails out without any call to spi_cleanup() and the same
memory will leak (Is this intended?).

Basically, fsl_spi_setup allocates memory using devm_kzalloc, while
device_add expects that any memory allocated via this way is only
done in the device's probe function.

The simple fix would be to do a normal allocation (revert the patch) and
add a free to the cleanup() function. Unfortunately that doesn't fix the
memleak I mentioned above.

So, some outside-the-box thinking brings me to conclude that another way
to fix this problem is to allocate the devm_kzalloc not on the device's
resource
list but on the controller's resources (it's controller state after all...).

Regards,
Stijn
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index b3e7775..98ccd23 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -431,7 +431,7 @@  static int fsl_spi_setup(struct spi_device *spi)
  		return -EINVAL;
  
  	if (!cs) {
-		cs = kzalloc(sizeof *cs, GFP_KERNEL);
+		cs = devm_kzalloc(&spi->dev, sizeof(*cs), GFP_KERNEL);
  		if (!cs)
  			return -ENOMEM;
  		spi->controller_state = cs;