* [lm-sensors] [PATCH 0/2] Accelerometer driver unload @ 2010-08-27 14:33 ` Yuri Ershov 0 siblings, 0 replies; 14+ messages in thread From: Yuri Ershov @ 2010-08-27 14:33 UTC (permalink / raw) To: eric.piel, samu.p.onkalo, akpm, daniel Cc: ext-yuri.kululin, lm-sensors, linux-kernel, ext-yuri.ershov Hello, I2C part of lis3 accelerometer driver can be unloaded while in use which may cause kernel oops. These patches allow to change /dev/freefall file ops owner to prevent driver unloading while device in use. Please review. Best regards, Yuri Yuri Kululin (2): lis3: Add device owner lis3: Set device owner drivers/hwmon/lis3lv02d.c | 8 ++++++-- drivers/hwmon/lis3lv02d.h | 2 ++ drivers/hwmon/lis3lv02d_i2c.c | 2 ++ 3 files changed, 10 insertions(+), 2 deletions(-) _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/2] Accelerometer driver unload @ 2010-08-27 14:33 ` Yuri Ershov 0 siblings, 0 replies; 14+ messages in thread From: Yuri Ershov @ 2010-08-27 14:33 UTC (permalink / raw) To: eric.piel, samu.p.onkalo, akpm, daniel Cc: ext-yuri.kululin, lm-sensors, linux-kernel, ext-yuri.ershov Hello, I2C part of lis3 accelerometer driver can be unloaded while in use which may cause kernel oops. These patches allow to change /dev/freefall file ops owner to prevent driver unloading while device in use. Please review. Best regards, Yuri Yuri Kululin (2): lis3: Add device owner lis3: Set device owner drivers/hwmon/lis3lv02d.c | 8 ++++++-- drivers/hwmon/lis3lv02d.h | 2 ++ drivers/hwmon/lis3lv02d_i2c.c | 2 ++ 3 files changed, 10 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [lm-sensors] [PATCH 1/2] lis3: Add device owner 2010-08-27 14:33 ` Yuri Ershov @ 2010-08-27 14:33 ` Yuri Ershov -1 siblings, 0 replies; 14+ messages in thread From: Yuri Ershov @ 2010-08-27 14:33 UTC (permalink / raw) To: eric.piel, samu.p.onkalo, akpm, daniel Cc: ext-yuri.kululin, lm-sensors, linux-kernel, ext-yuri.ershov Add device owner and change /dev/freefall file operations owner according to the used driver Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> --- drivers/hwmon/lis3lv02d.c | 8 ++++++-- drivers/hwmon/lis3lv02d.h | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c index e278f0e..0f1bd36 100644 --- a/drivers/hwmon/lis3lv02d.c +++ b/drivers/hwmon/lis3lv02d.c @@ -591,8 +591,7 @@ static int lis3lv02d_misc_fasync(int fd, struct file *file, int on) return fasync_helper(fd, file, on, &lis3_dev.async_queue); } -static const struct file_operations lis3lv02d_misc_fops = { - .owner = THIS_MODULE, +static struct file_operations lis3lv02d_misc_fops = { .llseek = no_llseek, .read = lis3lv02d_misc_read, .open = lis3lv02d_misc_open, @@ -1007,6 +1006,11 @@ int lis3lv02d_init_device(struct lis3lv02d *dev) goto out; } + if (dev->owner) + lis3lv02d_misc_fops.owner = dev->owner; + else + lis3lv02d_misc_fops.owner = THIS_MODULE; + if (misc_register(&lis3lv02d_misc_device)) printk(KERN_ERR DRIVER_NAME ": misc_register failed\n"); out: diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h index 1f0a66f..e63dd3d 100644 --- a/drivers/hwmon/lis3lv02d.h +++ b/drivers/hwmon/lis3lv02d.h @@ -258,6 +258,8 @@ struct lis3lv02d { unsigned char irq_cfg; struct lis3lv02d_platform_data *pdata; /* for passing board config */ struct mutex mutex; /* Serialize poll and selftest */ + + struct module *owner; /* Device owner */ }; int lis3lv02d_init_device(struct lis3lv02d *lis3); -- 1.7.1.1 _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/2] lis3: Add device owner @ 2010-08-27 14:33 ` Yuri Ershov 0 siblings, 0 replies; 14+ messages in thread From: Yuri Ershov @ 2010-08-27 14:33 UTC (permalink / raw) To: eric.piel, samu.p.onkalo, akpm, daniel Cc: ext-yuri.kululin, lm-sensors, linux-kernel, ext-yuri.ershov Add device owner and change /dev/freefall file operations owner according to the used driver Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> --- drivers/hwmon/lis3lv02d.c | 8 ++++++-- drivers/hwmon/lis3lv02d.h | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c index e278f0e..0f1bd36 100644 --- a/drivers/hwmon/lis3lv02d.c +++ b/drivers/hwmon/lis3lv02d.c @@ -591,8 +591,7 @@ static int lis3lv02d_misc_fasync(int fd, struct file *file, int on) return fasync_helper(fd, file, on, &lis3_dev.async_queue); } -static const struct file_operations lis3lv02d_misc_fops = { - .owner = THIS_MODULE, +static struct file_operations lis3lv02d_misc_fops = { .llseek = no_llseek, .read = lis3lv02d_misc_read, .open = lis3lv02d_misc_open, @@ -1007,6 +1006,11 @@ int lis3lv02d_init_device(struct lis3lv02d *dev) goto out; } + if (dev->owner) + lis3lv02d_misc_fops.owner = dev->owner; + else + lis3lv02d_misc_fops.owner = THIS_MODULE; + if (misc_register(&lis3lv02d_misc_device)) printk(KERN_ERR DRIVER_NAME ": misc_register failed\n"); out: diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h index 1f0a66f..e63dd3d 100644 --- a/drivers/hwmon/lis3lv02d.h +++ b/drivers/hwmon/lis3lv02d.h @@ -258,6 +258,8 @@ struct lis3lv02d { unsigned char irq_cfg; struct lis3lv02d_platform_data *pdata; /* for passing board config */ struct mutex mutex; /* Serialize poll and selftest */ + + struct module *owner; /* Device owner */ }; int lis3lv02d_init_device(struct lis3lv02d *lis3); -- 1.7.1.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [lm-sensors] [PATCH 2/2] lis3: Set device owner 2010-08-27 14:33 ` Yuri Ershov @ 2010-08-27 14:33 ` Yuri Ershov -1 siblings, 0 replies; 14+ messages in thread From: Yuri Ershov @ 2010-08-27 14:33 UTC (permalink / raw) To: eric.piel, samu.p.onkalo, akpm, daniel Cc: ext-yuri.kululin, lm-sensors, linux-kernel, ext-yuri.ershov Set device owner for i2c part. Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> --- drivers/hwmon/lis3lv02d_i2c.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/hwmon/lis3lv02d_i2c.c b/drivers/hwmon/lis3lv02d_i2c.c index b2f8bae..1a55bb3 100644 --- a/drivers/hwmon/lis3lv02d_i2c.c +++ b/drivers/hwmon/lis3lv02d_i2c.c @@ -129,6 +129,7 @@ static int __devinit lis3lv02d_i2c_probe(struct i2c_client *client, lis3_dev.reg_ctrl = lis3_reg_ctrl; lis3_dev.irq = client->irq; lis3_dev.ac = lis3lv02d_axis_map; + lis3_dev.owner = THIS_MODULE; i2c_set_clientdata(client, &lis3_dev); @@ -149,6 +150,7 @@ static int __devexit lis3lv02d_i2c_remove(struct i2c_client *client) lis3lv02d_joystick_disable(); lis3lv02d_remove_fs(&lis3_dev); + lis3_dev.owner = NULL; regulator_bulk_free(ARRAY_SIZE(lis3_dev.regulators), lis3_dev.regulators); -- 1.7.1.1 _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] lis3: Set device owner @ 2010-08-27 14:33 ` Yuri Ershov 0 siblings, 0 replies; 14+ messages in thread From: Yuri Ershov @ 2010-08-27 14:33 UTC (permalink / raw) To: eric.piel, samu.p.onkalo, akpm, daniel Cc: ext-yuri.kululin, lm-sensors, linux-kernel, ext-yuri.ershov Set device owner for i2c part. Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> --- drivers/hwmon/lis3lv02d_i2c.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/hwmon/lis3lv02d_i2c.c b/drivers/hwmon/lis3lv02d_i2c.c index b2f8bae..1a55bb3 100644 --- a/drivers/hwmon/lis3lv02d_i2c.c +++ b/drivers/hwmon/lis3lv02d_i2c.c @@ -129,6 +129,7 @@ static int __devinit lis3lv02d_i2c_probe(struct i2c_client *client, lis3_dev.reg_ctrl = lis3_reg_ctrl; lis3_dev.irq = client->irq; lis3_dev.ac = lis3lv02d_axis_map; + lis3_dev.owner = THIS_MODULE; i2c_set_clientdata(client, &lis3_dev); @@ -149,6 +150,7 @@ static int __devexit lis3lv02d_i2c_remove(struct i2c_client *client) lis3lv02d_joystick_disable(); lis3lv02d_remove_fs(&lis3_dev); + lis3_dev.owner = NULL; regulator_bulk_free(ARRAY_SIZE(lis3_dev.regulators), lis3_dev.regulators); -- 1.7.1.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [2/2] lis3: Set device owner 2010-08-27 14:33 ` Yuri Ershov @ 2010-09-24 20:27 ` Guenter Roeck -1 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2010-09-24 20:27 UTC (permalink / raw) To: Yuri Ershov Cc: eric.piel, samu.p.onkalo, akpm, daniel, ext-yuri.kululin, lm-sensors, linux-kernel On Fri, Aug 27, 2010 at 02:33:21PM -0000, Yuri Ershov wrote: > Set device owner for i2c part. > > Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> > > --- > drivers/hwmon/lis3lv02d_i2c.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/hwmon/lis3lv02d_i2c.c b/drivers/hwmon/lis3lv02d_i2c.c > index b2f8bae..1a55bb3 100644 > --- a/drivers/hwmon/lis3lv02d_i2c.c > +++ b/drivers/hwmon/lis3lv02d_i2c.c > @@ -129,6 +129,7 @@ static int __devinit lis3lv02d_i2c_probe(struct i2c_client *client, > lis3_dev.reg_ctrl = lis3_reg_ctrl; > lis3_dev.irq = client->irq; > lis3_dev.ac = lis3lv02d_axis_map; > + lis3_dev.owner = THIS_MODULE; > > i2c_set_clientdata(client, &lis3_dev); > > @@ -149,6 +150,7 @@ static int __devexit lis3lv02d_i2c_remove(struct i2c_client *client) > > lis3lv02d_joystick_disable(); > lis3lv02d_remove_fs(&lis3_dev); > + lis3_dev.owner = NULL; > Should not be necessary, since you are about to unload the driver. Unless I am missing something, of course. Guenter _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [2/2] lis3: Set device owner @ 2010-09-24 20:27 ` Guenter Roeck 0 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2010-09-24 20:27 UTC (permalink / raw) To: Yuri Ershov Cc: eric.piel, samu.p.onkalo, akpm, daniel, ext-yuri.kululin, lm-sensors, linux-kernel On Fri, Aug 27, 2010 at 02:33:21PM -0000, Yuri Ershov wrote: > Set device owner for i2c part. > > Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> > > --- > drivers/hwmon/lis3lv02d_i2c.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/hwmon/lis3lv02d_i2c.c b/drivers/hwmon/lis3lv02d_i2c.c > index b2f8bae..1a55bb3 100644 > --- a/drivers/hwmon/lis3lv02d_i2c.c > +++ b/drivers/hwmon/lis3lv02d_i2c.c > @@ -129,6 +129,7 @@ static int __devinit lis3lv02d_i2c_probe(struct i2c_client *client, > lis3_dev.reg_ctrl = lis3_reg_ctrl; > lis3_dev.irq = client->irq; > lis3_dev.ac = lis3lv02d_axis_map; > + lis3_dev.owner = THIS_MODULE; > > i2c_set_clientdata(client, &lis3_dev); > > @@ -149,6 +150,7 @@ static int __devexit lis3lv02d_i2c_remove(struct i2c_client *client) > > lis3lv02d_joystick_disable(); > lis3lv02d_remove_fs(&lis3_dev); > + lis3_dev.owner = NULL; > Should not be necessary, since you are about to unload the driver. Unless I am missing something, of course. Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [1/2] lis3: Add device owner 2010-08-27 14:33 ` Yuri Ershov @ 2010-09-24 20:24 ` Guenter Roeck -1 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2010-09-24 20:24 UTC (permalink / raw) To: Yuri Ershov Cc: eric.piel, samu.p.onkalo, akpm, daniel, ext-yuri.kululin, lm-sensors, linux-kernel On Fri, Aug 27, 2010 at 02:33:20PM -0000, Yuri Ershov wrote: > Add device owner and change /dev/freefall file operations owner > according to the used driver > > Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> > > --- > drivers/hwmon/lis3lv02d.c | 8 ++++++-- > drivers/hwmon/lis3lv02d.h | 2 ++ > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c > index e278f0e..0f1bd36 100644 > --- a/drivers/hwmon/lis3lv02d.c > +++ b/drivers/hwmon/lis3lv02d.c > @@ -591,8 +591,7 @@ static int lis3lv02d_misc_fasync(int fd, struct file *file, int on) > return fasync_helper(fd, file, on, &lis3_dev.async_queue); > } > > -static const struct file_operations lis3lv02d_misc_fops = { > - .owner = THIS_MODULE, > +static struct file_operations lis3lv02d_misc_fops = { > .llseek = no_llseek, > .read = lis3lv02d_misc_read, > .open = lis3lv02d_misc_open, > @@ -1007,6 +1006,11 @@ int lis3lv02d_init_device(struct lis3lv02d *dev) > goto out; > } > > + if (dev->owner) > + lis3lv02d_misc_fops.owner = dev->owner; > + else > + lis3lv02d_misc_fops.owner = THIS_MODULE; > + If you retained the static assignment to .owner above, you would not need the else part here. Also, this depends on patch#2, which actually sets dev->owner. So the problem isn't really addressed w/o patch #2. Given that, I am not sure if it makes sense to have two separate patches. If both patches were applied as one, you might not need the if () above in the first place. And if you do, you might still not have solved the problem completely since .owner would not be set correctly if dev->owner is NULL. On a higher level, reassigning the owner like this seems to be quite uncommon, at least in hwmon. I would like to see an Acked-by from Eric to ensure that the fix is correct. Thanks, Guenter _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [1/2] lis3: Add device owner @ 2010-09-24 20:24 ` Guenter Roeck 0 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2010-09-24 20:24 UTC (permalink / raw) To: Yuri Ershov Cc: eric.piel, samu.p.onkalo, akpm, daniel, ext-yuri.kululin, lm-sensors, linux-kernel On Fri, Aug 27, 2010 at 02:33:20PM -0000, Yuri Ershov wrote: > Add device owner and change /dev/freefall file operations owner > according to the used driver > > Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> > > --- > drivers/hwmon/lis3lv02d.c | 8 ++++++-- > drivers/hwmon/lis3lv02d.h | 2 ++ > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c > index e278f0e..0f1bd36 100644 > --- a/drivers/hwmon/lis3lv02d.c > +++ b/drivers/hwmon/lis3lv02d.c > @@ -591,8 +591,7 @@ static int lis3lv02d_misc_fasync(int fd, struct file *file, int on) > return fasync_helper(fd, file, on, &lis3_dev.async_queue); > } > > -static const struct file_operations lis3lv02d_misc_fops = { > - .owner = THIS_MODULE, > +static struct file_operations lis3lv02d_misc_fops = { > .llseek = no_llseek, > .read = lis3lv02d_misc_read, > .open = lis3lv02d_misc_open, > @@ -1007,6 +1006,11 @@ int lis3lv02d_init_device(struct lis3lv02d *dev) > goto out; > } > > + if (dev->owner) > + lis3lv02d_misc_fops.owner = dev->owner; > + else > + lis3lv02d_misc_fops.owner = THIS_MODULE; > + If you retained the static assignment to .owner above, you would not need the else part here. Also, this depends on patch#2, which actually sets dev->owner. So the problem isn't really addressed w/o patch #2. Given that, I am not sure if it makes sense to have two separate patches. If both patches were applied as one, you might not need the if () above in the first place. And if you do, you might still not have solved the problem completely since .owner would not be set correctly if dev->owner is NULL. On a higher level, reassigning the owner like this seems to be quite uncommon, at least in hwmon. I would like to see an Acked-by from Eric to ensure that the fix is correct. Thanks, Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [1/2] lis3: Add device owner 2010-09-24 20:24 ` Guenter Roeck @ 2010-09-28 8:52 ` Yuri Kululin -1 siblings, 0 replies; 14+ messages in thread From: Yuri Kululin @ 2010-09-28 8:52 UTC (permalink / raw) To: ext Guenter Roeck Cc: Yuri Ershov, eric.piel, samu.p.onkalo, akpm, daniel, lm-sensors, linux-kernel ext Guenter Roeck wrote: > On Fri, Aug 27, 2010 at 02:33:20PM -0000, Yuri Ershov wrote: >> Add device owner and change /dev/freefall file operations owner >> according to the used driver >> >> Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> >> >> --- >> drivers/hwmon/lis3lv02d.c | 8 ++++++-- >> drivers/hwmon/lis3lv02d.h | 2 ++ >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c >> index e278f0e..0f1bd36 100644 >> --- a/drivers/hwmon/lis3lv02d.c >> +++ b/drivers/hwmon/lis3lv02d.c >> @@ -591,8 +591,7 @@ static int lis3lv02d_misc_fasync(int fd, struct file *file, int on) >> return fasync_helper(fd, file, on, &lis3_dev.async_queue); >> } >> >> -static const struct file_operations lis3lv02d_misc_fops = { >> - .owner = THIS_MODULE, >> +static struct file_operations lis3lv02d_misc_fops = { >> .llseek = no_llseek, >> .read = lis3lv02d_misc_read, >> .open = lis3lv02d_misc_open, >> @@ -1007,6 +1006,11 @@ int lis3lv02d_init_device(struct lis3lv02d *dev) >> goto out; >> } >> >> + if (dev->owner) >> + lis3lv02d_misc_fops.owner = dev->owner; >> + else >> + lis3lv02d_misc_fops.owner = THIS_MODULE; >> + > If you retained the static assignment to .owner above, you would not need > the else part here. Yes, you are right. I can change this. > Also, this depends on patch#2, which actually sets dev->owner. > So the problem isn't really addressed w/o patch #2. Given that, > I am not sure if it makes sense to have two separate patches. Ok, I can merge my patches. > If both patches were applied as one, you might not need the if () > above in the first place. And if you do, you might still not have solved > the problem completely since .owner would not be set correctly > if dev->owner is NULL. I've added "if" here to keep the situation "as is" for all other drivers (HP driver - hp_accel.c and spi part - lis3lv02d_spi.c. I don't have the HW to test) which are using the lis3 core. If we want to avoid the issues with unloading completely all parts should set the dev->owner. Not a problem to add this to HP driver and lis3lv02d_spi.c. > On a higher level, reassigning the owner like this seems to be quite uncommon, > at least in hwmon. I would like to see an Acked-by from Eric to ensure > that the fix is correct. I tried to use the standard way of module usage control because all functions provided by lis3lv02d_i2c.c, lis3lv02d_spi.c or hp_accel.c can be called through lis3lv02d_misc_fops file ops or joystick poll device. I can propose another solution. We can use try_module_get(owner) during device registration (in lis3lv02d_add_fs()), release module by using module_put(owner) after device unregister (in lis3lv02d_remove_fs()) and do not touch lis3lv02d_misc_fops. But anyway dev->owner should be set. Thanks a lot for reviewing, Yuri K > > Thanks, > Guenter _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [1/2] lis3: Add device owner @ 2010-09-28 8:52 ` Yuri Kululin 0 siblings, 0 replies; 14+ messages in thread From: Yuri Kululin @ 2010-09-28 8:52 UTC (permalink / raw) To: ext Guenter Roeck Cc: Yuri Ershov, eric.piel, samu.p.onkalo, akpm, daniel, lm-sensors, linux-kernel ext Guenter Roeck wrote: > On Fri, Aug 27, 2010 at 02:33:20PM -0000, Yuri Ershov wrote: >> Add device owner and change /dev/freefall file operations owner >> according to the used driver >> >> Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com> >> >> --- >> drivers/hwmon/lis3lv02d.c | 8 ++++++-- >> drivers/hwmon/lis3lv02d.h | 2 ++ >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c >> index e278f0e..0f1bd36 100644 >> --- a/drivers/hwmon/lis3lv02d.c >> +++ b/drivers/hwmon/lis3lv02d.c >> @@ -591,8 +591,7 @@ static int lis3lv02d_misc_fasync(int fd, struct file *file, int on) >> return fasync_helper(fd, file, on, &lis3_dev.async_queue); >> } >> >> -static const struct file_operations lis3lv02d_misc_fops = { >> - .owner = THIS_MODULE, >> +static struct file_operations lis3lv02d_misc_fops = { >> .llseek = no_llseek, >> .read = lis3lv02d_misc_read, >> .open = lis3lv02d_misc_open, >> @@ -1007,6 +1006,11 @@ int lis3lv02d_init_device(struct lis3lv02d *dev) >> goto out; >> } >> >> + if (dev->owner) >> + lis3lv02d_misc_fops.owner = dev->owner; >> + else >> + lis3lv02d_misc_fops.owner = THIS_MODULE; >> + > If you retained the static assignment to .owner above, you would not need > the else part here. Yes, you are right. I can change this. > Also, this depends on patch#2, which actually sets dev->owner. > So the problem isn't really addressed w/o patch #2. Given that, > I am not sure if it makes sense to have two separate patches. Ok, I can merge my patches. > If both patches were applied as one, you might not need the if () > above in the first place. And if you do, you might still not have solved > the problem completely since .owner would not be set correctly > if dev->owner is NULL. I've added "if" here to keep the situation "as is" for all other drivers (HP driver - hp_accel.c and spi part - lis3lv02d_spi.c. I don't have the HW to test) which are using the lis3 core. If we want to avoid the issues with unloading completely all parts should set the dev->owner. Not a problem to add this to HP driver and lis3lv02d_spi.c. > On a higher level, reassigning the owner like this seems to be quite uncommon, > at least in hwmon. I would like to see an Acked-by from Eric to ensure > that the fix is correct. I tried to use the standard way of module usage control because all functions provided by lis3lv02d_i2c.c, lis3lv02d_spi.c or hp_accel.c can be called through lis3lv02d_misc_fops file ops or joystick poll device. I can propose another solution. We can use try_module_get(owner) during device registration (in lis3lv02d_add_fs()), release module by using module_put(owner) after device unregister (in lis3lv02d_remove_fs()) and do not touch lis3lv02d_misc_fops. But anyway dev->owner should be set. Thanks a lot for reviewing, Yuri K > > Thanks, > Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [1/2] lis3: Add device owner 2010-09-28 8:52 ` Yuri Kululin @ 2010-10-02 0:03 ` Guenter Roeck -1 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2010-10-02 0:03 UTC (permalink / raw) To: Yuri Kululin Cc: Yuri Ershov, eric.piel@tremplin-utc.net, samu.p.onkalo@nokia.com, akpm@linux-foundation.org, daniel@caiaq.de, lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org On Tue, Sep 28, 2010 at 04:52:19AM -0400, Yuri Kululin wrote: [ ... ] > > > On a higher level, reassigning the owner like this seems to be quite uncommon, > > at least in hwmon. I would like to see an Acked-by from Eric to ensure > > that the fix is correct. > > I tried to use the standard way of module usage control because all functions > provided by lis3lv02d_i2c.c, lis3lv02d_spi.c or hp_accel.c can be called through > lis3lv02d_misc_fops file ops or joystick poll device. > > I can propose another solution. We can use try_module_get(owner) during device > registration (in lis3lv02d_add_fs()), release module by using module_put(owner) > after device unregister (in lis3lv02d_remove_fs()) and do not touch > lis3lv02d_misc_fops. But anyway dev->owner should be set. > Key is really to get feedback from others, since I am neither familiar with module ownership handling nor with this driver. Thanks, Guenter _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [1/2] lis3: Add device owner @ 2010-10-02 0:03 ` Guenter Roeck 0 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2010-10-02 0:03 UTC (permalink / raw) To: Yuri Kululin Cc: Yuri Ershov, eric.piel@tremplin-utc.net, samu.p.onkalo@nokia.com, akpm@linux-foundation.org, daniel@caiaq.de, lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org On Tue, Sep 28, 2010 at 04:52:19AM -0400, Yuri Kululin wrote: [ ... ] > > > On a higher level, reassigning the owner like this seems to be quite uncommon, > > at least in hwmon. I would like to see an Acked-by from Eric to ensure > > that the fix is correct. > > I tried to use the standard way of module usage control because all functions > provided by lis3lv02d_i2c.c, lis3lv02d_spi.c or hp_accel.c can be called through > lis3lv02d_misc_fops file ops or joystick poll device. > > I can propose another solution. We can use try_module_get(owner) during device > registration (in lis3lv02d_add_fs()), release module by using module_put(owner) > after device unregister (in lis3lv02d_remove_fs()) and do not touch > lis3lv02d_misc_fops. But anyway dev->owner should be set. > Key is really to get feedback from others, since I am neither familiar with module ownership handling nor with this driver. Thanks, Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-10-02 0:04 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-08-27 14:33 [lm-sensors] [PATCH 0/2] Accelerometer driver unload Yuri Ershov 2010-08-27 14:33 ` Yuri Ershov 2010-08-27 14:33 ` [lm-sensors] [PATCH 1/2] lis3: Add device owner Yuri Ershov 2010-08-27 14:33 ` Yuri Ershov 2010-08-27 14:33 ` [lm-sensors] [PATCH 2/2] lis3: Set " Yuri Ershov 2010-08-27 14:33 ` Yuri Ershov 2010-09-24 20:27 ` [lm-sensors] [2/2] " Guenter Roeck 2010-09-24 20:27 ` Guenter Roeck 2010-09-24 20:24 ` [lm-sensors] [1/2] lis3: Add " Guenter Roeck 2010-09-24 20:24 ` Guenter Roeck 2010-09-28 8:52 ` [lm-sensors] " Yuri Kululin 2010-09-28 8:52 ` Yuri Kululin 2010-10-02 0:03 ` [lm-sensors] " Guenter Roeck 2010-10-02 0:03 ` Guenter Roeck
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.