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

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

[Uwe Kleine-König]

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(-)

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
-- 
2.43.0

Re: [PATCH] media: i2c: et8ek8: Don't strip remove function when driver is builtin

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] media: i2c: et8ek8: Don't strip remove function when driver is builtin

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]

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.

Sorry for the noise
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] media: i2c: et8ek8: Don't strip remove function when driver is builtin

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

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

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] media: i2c: et8ek8: Don't strip remove function when driver is builtin

[Sakari Ailus]

Hi Uwe,

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). 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.

-- 
Regards,

Sakari Ailus

Re: [PATCH] media: i2c: et8ek8: Don't strip remove function when driver is builtin

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2356 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]