diff mbox series

media: i2c: et8ek8: Don't strip remove function when driver is builtin

Message ID 20240324160045.238647-2-u.kleine-koenig@pengutronix.de
State Accepted
Commit 545b215736c5c4b354e182d99c578a472ac9bfce
Headers show
Series media: i2c: et8ek8: Don't strip remove function when driver is builtin | expand

Commit Message

Uwe Kleine-König March 24, 2024, 4 p.m. UTC
Using __exit for the remove function results in the remove callback
being discarded with CONFIG_VIDEO_ET8EK8=y. When such a device gets
unbound (e.g. using sysfs or hotplug), the driver is just removed
without the cleanup being performed. This results in resource leaks. Fix
it by compiling in the remove callback unconditionally.

This also fixes a W=1 modpost warning:

	WARNING: modpost: drivers/media/i2c/et8ek8/et8ek8: section mismatch in reference: et8ek8_i2c_driver+0x10 (section: .data) -> et8ek8_remove (section: .exit.text)

Fixes: c5254e72b8ed ("[media] media: Driver for Toshiba et8ek8 5MP sensor")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/media/i2c/et8ek8/et8ek8_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 70293240c5ce675a67bfc48f419b093023b862b3

Comments

Uwe Kleine-König April 29, 2024, 8:20 p.m. UTC | #1
Hello,

On Sun, Mar 24, 2024 at 05:00:44PM +0100, Uwe Kleine-König wrote:
> Using __exit for the remove function results in the remove callback
> being discarded with CONFIG_VIDEO_ET8EK8=y. When such a device gets
> unbound (e.g. using sysfs or hotplug), the driver is just removed
> without the cleanup being performed. This results in resource leaks. Fix
> it by compiling in the remove callback unconditionally.
> 
> This also fixes a W=1 modpost warning:
> 
> 	WARNING: modpost: drivers/media/i2c/et8ek8/et8ek8: section mismatch in reference: et8ek8_i2c_driver+0x10 (section: .data) -> et8ek8_remove (section: .exit.text)
> 
> Fixes: c5254e72b8ed ("[media] media: Driver for Toshiba et8ek8 5MP sensor")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I wonder if I failed to make the commit log drastic enough as the patch
wasn't picked up yet. This is a fix for a resource leak and IMHO should
qualify to go in before v6.9 (though I admit it gets late for that).

Did I address the right people?

Best regards
Uwe
Pavel Machek April 29, 2024, 8:35 p.m. UTC | #2
On Sun 2024-03-24 17:00:44, Uwe Kleine-König wrote:
> Using __exit for the remove function results in the remove callback
> being discarded with CONFIG_VIDEO_ET8EK8=y. When such a device gets
> unbound (e.g. using sysfs or hotplug), the driver is just removed
> without the cleanup being performed. This results in resource leaks. Fix
> it by compiling in the remove callback unconditionally.
> 
> This also fixes a W=1 modpost warning:
> 
> 	WARNING: modpost: drivers/media/i2c/et8ek8/et8ek8: section mismatch in reference: et8ek8_i2c_driver+0x10 (section: .data) -> et8ek8_remove (section: .exit.text)
> 
> Fixes: c5254e72b8ed ("[media] media: Driver for Toshiba et8ek8 5MP sensor")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Pavel Machek <pavel@ucw.cz>

You might want to cc akpm if this does not get picked up.

Best regards,
									Pavel
									
> ---
>  drivers/media/i2c/et8ek8/et8ek8_driver.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> index f548b1bb75fb..e932d25ca7b3 100644
> --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> @@ -1475,7 +1475,7 @@ static int et8ek8_probe(struct i2c_client *client)
>  	return ret;
>  }
>  
> -static void __exit et8ek8_remove(struct i2c_client *client)
> +static void et8ek8_remove(struct i2c_client *client)
>  {
>  	struct v4l2_subdev *subdev = i2c_get_clientdata(client);
>  	struct et8ek8_sensor *sensor = to_et8ek8_sensor(subdev);
> @@ -1517,7 +1517,7 @@ static struct i2c_driver et8ek8_i2c_driver = {
>  		.of_match_table	= et8ek8_of_table,
>  	},
>  	.probe		= et8ek8_probe,
> -	.remove		= __exit_p(et8ek8_remove),
> +	.remove		= et8ek8_remove,
>  	.id_table	= et8ek8_id_table,
>  };
>  
> 
> base-commit: 70293240c5ce675a67bfc48f419b093023b862b3
Uwe Kleine-König April 30, 2024, 5:04 p.m. UTC | #3
Hello Sakari,

On Mon, Apr 29, 2024 at 09:43:09PM +0000, Sakari Ailus wrote:
> On Mon, Apr 29, 2024 at 10:26:55PM +0200, Uwe Kleine-König wrote:
> > On Mon, Apr 29, 2024 at 10:20:09PM +0200, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > On Sun, Mar 24, 2024 at 05:00:44PM +0100, Uwe Kleine-König wrote:
> > > > Using __exit for the remove function results in the remove callback
> > > > being discarded with CONFIG_VIDEO_ET8EK8=y. When such a device gets
> > > > unbound (e.g. using sysfs or hotplug), the driver is just removed
> > > > without the cleanup being performed. This results in resource leaks. Fix
> > > > it by compiling in the remove callback unconditionally.
> > > > 
> > > > This also fixes a W=1 modpost warning:
> > > > 
> > > > 	WARNING: modpost: drivers/media/i2c/et8ek8/et8ek8: section mismatch in reference: et8ek8_i2c_driver+0x10 (section: .data) -> et8ek8_remove (section: .exit.text)
> > > > 
> > > > Fixes: c5254e72b8ed ("[media] media: Driver for Toshiba et8ek8 5MP sensor")
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > 
> > > I wonder if I failed to make the commit log drastic enough as the patch
> > > wasn't picked up yet. This is a fix for a resource leak and IMHO should
> > > qualify to go in before v6.9 (though I admit it gets late for that).
> > > 
> > > Did I address the right people?
> > 
> > Oh, I fatfingered my git cmdline and so missed this patch is in next
> > already. I still think getting it into v6.9 would have been nice, but I
> > won't argue if it goes into v6.10-rc1.
> 
> You should have received an e-mail from Patchwork when it got applied to my
> tree (or around that time).

Oh, I did indeed. It would be great (at least for my workflow) if these
notifications had an In-Reply-To header to thread with the original
mail. Hmm, maybe that's not so easy as the notification might contain
information about >1 patch. Hmm.

> It may still take a long time for it to be in linux-next and that's of
> course quite confusing. That will change eventually as we're amidst
> changing the process but we're not there yet.

Thanks for the heads-up.

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index f548b1bb75fb..e932d25ca7b3 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -1475,7 +1475,7 @@  static int et8ek8_probe(struct i2c_client *client)
 	return ret;
 }
 
-static void __exit et8ek8_remove(struct i2c_client *client)
+static void et8ek8_remove(struct i2c_client *client)
 {
 	struct v4l2_subdev *subdev = i2c_get_clientdata(client);
 	struct et8ek8_sensor *sensor = to_et8ek8_sensor(subdev);
@@ -1517,7 +1517,7 @@  static struct i2c_driver et8ek8_i2c_driver = {
 		.of_match_table	= et8ek8_of_table,
 	},
 	.probe		= et8ek8_probe,
-	.remove		= __exit_p(et8ek8_remove),
+	.remove		= et8ek8_remove,
 	.id_table	= et8ek8_id_table,
 };