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
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>
From:   zhangfei <zhangfei.gao@linaro.org>
Message-ID: <39f792df-e3e3-eaa6-f78b-bf325b79f1b7@linaro.org>
Date:   Fri, 30 Aug 2019 22:54:46 +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: <20190829095439.GA13915@kroah.com>
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/29 下午5:54, Greg Kroah-Hartman wrote:
> On Thu, Aug 29, 2019 at 05:05:13PM +0800, zhangfei wrote:
>> Hi, Greg
>>
>> 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" :)
>> Here the 'struct cdev' and 'struct device' have the same lifetime and
>> refcount.
> No they do not, that's impossible as refcounts are incremented from
> different places (i.e. userspace).
>
>> They are allocated with uacce when uacce_register and freed when
>> uacce_unregister.
> And that will not work.
I am sorry, could I ask more about this part.

   * This function should be used whenever the struct cdev and the
  * struct device are members of the same structure whose lifetime is
  * managed by the struct device.

 From cdev_device_add comments, looks struct cdev and stuct device
can be in the same structure like uacce, and uacce is released when
put_device(device)

Also cdev_device_del do the device_del(dev) and cdev_del(cdev).

Copy:
fs/char_dev.c
/**
  * cdev_device_add() - add a char device and it's corresponding
  *      struct device, linkink
  * @dev: the device structure
  * @cdev: the cdev structure
  *
  * cdev_device_add() adds the char device represented by @cdev to the 
system,
  * just as cdev_add does. It then adds @dev to the system using device_add
  * The dev_t for the char device will be taken from the struct device which
  * needs to be initialized first. This helper function correctly takes a
  * reference to the parent device so the parent will not get released until
  * all references to the cdev are released.
  *
  * This helper uses dev->devt for the device number. If it is not set
  * it will not add the cdev and it will be equivalent to device_add.
  *
  * This function should be used whenever the struct cdev and the
  * struct device are members of the same structure whose lifetime is
  * managed by the struct device.
  *
  * NOTE: Callers must assume that userspace was able to open the cdev and
  * can call cdev fops callbacks at any time, even if this function fails.
  */
int cdev_device_add(struct cdev *cdev, struct device *dev)
{
         int rc = 0;

         if (dev->devt) {
                 cdev_set_parent(cdev, &dev->kobj);

                 rc = cdev_add(cdev, dev->devt, 1);
                 if (rc)
                         return rc;
         }

         rc = device_add(dev);
         if (rc)
                 cdev_del(cdev);

         return rc;
}

Thanks