Re: [PATCH v4 14/38] platform/x86: intel_scu_ipc: Introduce new SCU IPC API

Mika Westerberg
2020-01-22T15:04:12Z

On Wed, Jan 22, 2020 at 03:49:39PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Jan 22, 2020 at 04:40:48PM +0200, Mika Westerberg wrote:
> > On Wed, Jan 22, 2020 at 02:43:59PM +0100, Greg Kroah-Hartman wrote:
> > > > + if (!scu->dev)
> > > > + goto err_unlock;
> > > > + if (!try_module_get(scu->dev->driver->owner))
> > > > + goto err_unlock;
> > > > + mutex_unlock(&ipclock);
> > > > + return scu;
> > >
> > > NO REFERENCE COUNT INCREMENT???
> >
> > You mean increment the scu->dev reference count? I kind of thought that
> > the try_module_get() should make sure the thing stays there as long as
> > the caller has not called _put() but now when I think about it bit more
> > we would need to do device_get() here as well.
>
> module reference counts handle _code_ while structure reference counts
> handle _data_.
>
> You should almost never need to worry about module reference counts if
> your code is structured properly, only handle the reference counts on
> the pointers you throw around.
>
> The fact that you are even calling try_module_get() is a huge flag that
> something is wrong here.

Thanks for the explanation! I'll keep this in mind and make sure the
next version has reference counting done accordingly.