On 13/06/2018 14:12, Cornelia Huck wrote:
On Wed, 13 Jun 2018 14:01:54 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 13/06/2018 13:14, Cornelia Huck wrote:
On Wed, 13 Jun 2018 12:54:40 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 13/06/2018 09:48, Cornelia Huck wrote:
On Wed, 13 Jun 2018 09:41:16 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 07/05/2018 17:11, Tony Krowiak wrote:
Introduces a new AP device driver. This device driver
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.
...snip...
+static int vfio_ap_matrix_dev_create(void)
+{
+ 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);
Did not see this before but here you certainly want to
do a kfree and not a put_device.
No, this must not be a kfree. Once you've tried to register something
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.
learned something again :) ,
but still, a kfree is needed for the kzalloc.
Does'nt it?
No, the put callback for the embedding structure needs to take care of
freeing things. Otherwise it is buggy.
Seems buggy to me.
How does the put_device knows if it is embedded and then in what it is
embedded ?
It does not need to know; the code registering the structure needs to
set up device->release correctly.
yes right, thanks.
+
+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);
Also here you need a kfree(ap_matrix) too .
Same here.
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
