Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2 2/2] uacce: add uacce driver
From:   zhangfei <zhangfei.gao@linaro.org>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Arnd Bergmann <arnd@arndb.de>, jonathan.cameron@huawei.com,
        kenneth-lee-2012@foxmail.com, Wangzhou <wangzhou1@hisilicon.com>,
        linux-accelerators@lists.ozlabs.org, linux-kernel@vger.kernel.org,
        Kenneth Lee <liguozhu@hisilicon.com>,
        Zaibo Xu <xuzaibo@huawei.com>
References: <1566998876-31770-1-git-send-email-zhangfei.gao@linaro.org>
 <1566998876-31770-3-git-send-email-zhangfei.gao@linaro.org>
 <20190828152201.GA10163@kroah.com>
 <5c2b0889-ea05-1ecd-fe5b-40611bd31945@linaro.org>
 <20190829095439.GA13915@kroah.com>
 <39f792df-e3e3-eaa6-f78b-bf325b79f1b7@linaro.org>
Message-ID: <eb9dd458-a82a-593a-1165-ff268ec995b0@linaro.org>
Date:   Mon, 2 Sep 2019 11:44:16 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <39f792df-e3e3-eaa6-f78b-bf325b79f1b7@linaro.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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.