Re: [PATCH v1 2/3] iio: add MEMSensing MSA311 3-axis accelerometer driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan,

I have two items to be discussed about iio_trigger_get().
Please see my questions below and correct me if I'm wrong.

On Tue, Apr 26, 2022 at 08:24:10PM +0300, Dmitry Rokosov wrote:
> > > +							       "%s-new-data",
> > > +							       indio_dev->name);
> > > +		if (!msa311->new_data_trig) {
> > > +			dev_err(&i2c->dev, "cannot allocate new data trig\n");
> > > +			err = -ENOMEM;
> > > +			goto err_lock_destroy;
> > > +		}
> > > +
> > > +		msa311->new_data_trig->dev.parent = &i2c->dev;
> > > +		msa311->new_data_trig->ops = &msa311_new_data_trig_ops;
> > > +		iio_trigger_set_drvdata(msa311->new_data_trig, indio_dev);
> > > +		indio_dev->trig = msa311->new_data_trig;
> > 
> > This will create a double free if you were to change the trigger.
> > 		indio_dev->trig = iio_trigger_get(trig);
> > 
> 
> I didn't take into account other trigger usage.
> I'll rework this place for the v2.
> 

The first one problem is module_get() calling for trigger get()
semantic.
I've applied iio_trigger_get() function to acquire module refcnt,
but I've faced with rmmod busy problem. IIO driver module doesn't want to
stop and unload due to not having zero module refcnt.
Syscall delete_module() tries to stop module first and after calls
driver exit() function (which executes devm_* handlers inside, including IIO
trigger unregister). It means we have the chicken or the egg dilemma here.
Module can't be unloaded until module refcnt is not zero and we can't
execute IIO trigger unregister (decrease module refcnt) only when module
refcnt is zero.
I suppose the possible solution to such a problem is a different semantic
for internal triggers (inside driver itself) and external drivers (like
hwtimer trigger). What do you think?

The second one issue is located in the different IIO drivers. Some modules
call iio_trigger_get() before iio_trigger_register(), trig->owner is not
initialized to the right value (THIS_MODULE) and we don't acquire refcnt
for proper driver object.
I'm going to send patchset to problem driver set, but I can test only
buildable status for such modules, are you okay with that?

-- 
Thank you,
Dmitry




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux