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

From: zhangfei
Date: Sun Sep 01 2019 - 23:44:50 EST

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

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 *'
uacce_destroy_chrdev: cdev_device_del->cdev_del(cdev)->kobject_put(&p->kobj);
if (refcount--) == 0

//Using 'struct device'
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.