Re: [PATCH v2 2/2] uacce: add uacce driver

[zhangfei]

Hi, Greg

On 2019/8/30 äå10:54, zhangfei wrote:
> > > On 2019/8/28 äå11:22, Greg Kroah-Hartman wrote:
> > > > On Wed, Aug 28, 2019 at 09:27:56PM +0800, Zhangfei Gao wrote:
> > > > > +struct uacce {
> > > > > +ÂÂÂ const char *drv_name;
> > > > > +ÂÂÂ const char *algs;
> > > > > +ÂÂÂ const char *api_ver;
> > > > > +ÂÂÂ unsigned int flags;
> > > > > +ÂÂÂ unsigned long qf_pg_start[UACCE_QFRT_MAX];
> > > > > +ÂÂÂ struct uacce_ops *ops;
> > > > > +ÂÂÂ struct device *pdev;
> > > > > +ÂÂÂ bool is_vf;
> > > > > +ÂÂÂ u32 dev_id;
> > > > > +ÂÂÂ struct cdev cdev;
> > > > > +ÂÂÂ struct device dev;
> > > > > +ÂÂÂ void *priv;
> > > > > +ÂÂÂ atomic_t state;
> > > > > +ÂÂÂ int prot;
> > > > > +ÂÂÂ struct mutex q_lock;
> > > > > +ÂÂÂ struct list_head qs;
> > > > > +};
> > > > At a quick glance, this problem really stood out to me. You CAN NOT
> > > > have two different objects within a structure that have different
> > > > lifetime rules and reference counts. You do that here with both a
> > > > 'struct cdev' and a 'struct device'. Pick one or the other, but never
> > > > both.
> > > >
> > > > I would recommend using a 'struct device' and then a 'struct cdev *'.
> > > > That way you get the advantage of using the driver model properly, and
> > > > then just adding your char device node pointer to "the side" which
> > > > interacts with this device.
> > > >
> > > > Then you might want to call this "struct uacce_device" :)
I think I understand now.
'struct device' and then a 'struct cdev' have different refcounts.
Using 'struct cdev *', the release is not in uacce.c, but controlled by 
cdev itself.
So uacce is decoupled with cdev.

//Using 'struct cdev *'
cdev_alloc->cdev_dynamic_release:kfree(p);
uacce_destroy_chrdev: 
cdev_device_del->cdev_del(cdev)->kobject_put(&p->kobj);
if (refcount--) == 0
cdev_dynamic_release->kfree(p);

//Using 'struct device'
cdev_init->cdev_default_release
cdev is freed in uacce.c,
So 'struct device' and then a 'struct cdev' are bind together, while 
cdev and uacce->dev have different refcount.

Thanks for the patience.