Re: [PATCH 04/22] ice: Register auxiliary device to provide RDMA
From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2021-01-25 19:11:33
Also in:
linux-rdma
On Fri, Jan 22, 2021 at 05:48:09PM -0600, Shiraz Saleem wrote:
+static void ice_peer_adev_release(struct device *dev)
+{
+ struct iidc_auxiliary_object *abo;
+ struct auxiliary_device *adev;
+
+ adev = container_of(dev, struct auxiliary_device, dev);
+ abo = container_of(adev, struct iidc_auxiliary_object, adev);This is just container_of(dev, struct iidc_auxiliary_object, adev.dev);
quoted hunk ↗ jump to hunk
@@ -1254,20 +1282,37 @@ int ice_init_peer_devices(struct ice_pf *pf) * |--> iidc_peer_obj * |--> *ice_peer_drv_int * + * iidc_auxiliary_object (container_of parent for adev) + * |--> auxiliary_device + * |--> *iidc_peer_obj (pointer from internal struct) + * * ice_peer_drv_int (internal only peer_drv struct) */ peer_obj_int = kzalloc(sizeof(*peer_obj_int), GFP_KERNEL); - if (!peer_obj_int) + if (!peer_obj_int) { + ida_simple_remove(&ice_peer_ida, id); return -ENOMEM; + }
Why is this allocated memory with a lifetime different from the aux device? This whole peer_dev/aux_dev split needs to go, why on earth does peer_obj need an entire state machine for driver binding? This is what the aux device and driver core or supposed to provide.
+ abo = kzalloc(sizeof(*abo), GFP_KERNEL);
+ if (!abo) {
+ ida_simple_remove(&ice_peer_ida, id);
+ kfree(peer_obj_int);
+ return -ENOMEM;
+ }Put the auxiliary_device_init() directly after kzalloc. Even better is to put everything up to the kzalloc/auxiliary_device_init() into a function called 'alloc_aux_device' Then all the error unwind here doesn't look so bad Jason