On 08/06/2015 01:14 PM, Matan Barak wrote: > Separate the cleanup and release IB cache functions. > The cleanup function delete all GIDs and unregister the event channel, > while the release function frees all memory. The cleanup function is > called after all clients were removed (in the device un-registration > process). > > The release function is called after the device was put. > This is in order to avoid use-after-free errors if ther vendor > driver's teardown code uses IB cache. > > Squash-into: 'IB/core: Add RoCE GID table management' > Signed-off-by: Matan Barak <matanb@xxxxxxxxxxxx> I had originally used the v1 of this patch instead of v2. In order to make sure I didn't miss anything, I hand compared the final results of the v1 patch to what this patch would have created if I had used it. I found a few things worth nothing, comments inline below. > @@ -903,20 +924,28 @@ int ib_cache_setup_one(struct ib_device *device) > (rdma_end_port(device) - > rdma_start_port(device) + 1), > GFP_KERNEL); > - err = gid_table_setup_one(device); > - > - if (!device->cache.pkey_cache || !device->cache.gid_cache || > + if (!device->cache.pkey_cache || > !device->cache.lmc_cache) { > printk(KERN_WARNING "Couldn't allocate cache " > "for %s\n", device->name); > - err = -ENOMEM; > - goto err; > + /* pkey_cache is freed here because we later access it's > + * elements. > + */ > + kfree(device->cache.pkey_cache); > + device->cache.pkey_cache = NULL; > + return -ENOMEM; This function is unnecessarily convoluted IMO. A simple change to using kzalloc() for the pkey_cache alloc eliminates part of the issue. As a result, I fixed this function up to be different than either patch. Please look over my results and make sure you agree with my changes. > } > > - for (p = 0; p <= rdma_end_port(device) - rdma_start_port(device); ++p) { > + for (p = 0; p <= rdma_end_port(device) - rdma_start_port(device); ++p) > device->cache.pkey_cache[p] = NULL; For instance this goes away entirely by using kzalloc(). The rest looked ok. I have to fixup the ordering changes between sysfs and cache to make the v1 patch match the v2 patch. -- Doug Ledford <dledford@xxxxxxxxxx> GPG KeyID: 0E572FDD
Attachment:
signature.asc
Description: OpenPGP digital signature