On Wed, 13 Jun 2018 14:01:54 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 13/06/2018 13:14, Cornelia Huck wrote:It does not need to know; the code registering the structure needs to
On Wed, 13 Jun 2018 12:54:40 +0200Seems buggy to me.
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 13/06/2018 09:48, Cornelia Huck wrote:No, the put callback for the embedding structure needs to take care of
On Wed, 13 Jun 2018 09:41:16 +0200learned something again :) ,
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 07/05/2018 17:11, Tony Krowiak wrote:No, this must not be a kfree. Once you've tried to register something
Introduces a new AP device driver. This device driver...snip...
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.
+static int vfio_ap_matrix_dev_create(void)Did not see this before but here you certainly want to
+{
+ int ret;
+
+ vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME);
+
+ if (IS_ERR(vfio_ap_root_device)) {
+ ret = PTR_ERR(vfio_ap_root_device);
+ goto done;
+ }
+
+ ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL);
+ if (!ap_matrix) {
+ ret = -ENOMEM;
+ goto matrix_alloc_err;
+ }
+
+ ap_matrix->device.type = &vfio_ap_dev_type;
+ dev_set_name(&ap_matrix->device, "%s", VFIO_AP_DEV_NAME);
+ ap_matrix->device.parent = vfio_ap_root_device;
+ ap_matrix->device.release = vfio_ap_matrix_dev_release;
+ ap_matrix->device.driver = &vfio_ap_drv.driver;
+
+ ret = device_register(&ap_matrix->device);
+ if (ret)
+ goto matrix_reg_err;
+
+ goto done;
+
+matrix_reg_err:
+ put_device(&ap_matrix->device);
do a kfree and not a put_device.
embedding a struct device with the driver core, you need to use
put_device, as another path may have acquired a reference, even if
registering ultimately failed. See the comment for device_register().
IOW, the code is correct.
but still, a kfree is needed for the kzalloc.
Does'nt it?
freeing things. Otherwise it is buggy.
How does the put_device knows if it is embedded and then in what it is
embedded ?
set up device->release correctly.
Same here.+Also here you need a kfree(ap_matrix) too .
+matrix_alloc_err:
+ root_device_unregister(vfio_ap_root_device);
+
+done:
+ return ret;
+}
+
+static void vfio_ap_matrix_dev_destroy(struct ap_matrix *ap_matrix)
+{
+ device_unregister(&ap_matrix->device);
+ root_device_unregister(vfio_ap_root_device);