| Message ID | 20240324160045.238647-2-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: et8ek8: Don't strip remove function when driver is builtin | expand |
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
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
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
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.
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 --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, };
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