On Mon, Dec 12, 2016 at 1:11 PM, Mark Bloch <markb@xxxxxxxxxxxx> wrote:
> Hi,
>
> On 12/12/2016 1:43 PM, Jinpu Wang wrote:
>> From fc80c1730d94a38934c5c60f9b9561745723dfd9 Mon Sep 17 00:00:00 2001
>> From: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxxx>
>> Date: Mon, 12 Dec 2016 09:52:42 +0100
>> Subject: [PATCH 1/4] IB/core: add port state cache
>>
>> We need a port state cache in ib_core, later we will use in rdma_cm.
>>
>> Signed-off-by: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxxx>
>> Reviewed-by: Michael Wang <yun.wang@xxxxxxxxxxxxxxxx>
>> ---
>> drivers/infiniband/core/cache.c | 9 ++++++++-
>> include/rdma/ib_verbs.h | 1 +
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
>> index 1a2984c..025db27 100644
>> --- a/drivers/infiniband/core/cache.c
>> +++ b/drivers/infiniband/core/cache.c
>> @@ -1109,6 +1109,8 @@ static void ib_cache_update(struct ib_device *device,
>> }
>>
>> device->cache.lmc_cache[port - rdma_start_port(device)] = tprops->lmc;
>> + device->cache.port_state_cache[port - rdma_start_port(device)] =
>> + tprops->state;
>>
>> write_unlock_irq(&device->cache.lock);
>>
>> @@ -1168,7 +1170,11 @@ int ib_cache_setup_one(struct ib_device *device)
>> (rdma_end_port(device) -
>> rdma_start_port(device) + 1),
>> GFP_KERNEL);
>> - if (!device->cache.pkey_cache ||
>> + device->cache.port_state_cache = kmalloc(sizeof
>> *device->cache.port_state_cache *
>> + (rdma_end_port(device) -
>> + rdma_start_port(device) + 1),
>> + GFP_KERNEL);
>> + if (!device->cache.pkey_cache || !device->cache.port_state_cache ||
> Looking at the code, I see we can leak memory here.
> if pkey_cache is allocated but lms_cache is not, we will return without freeing pkey_cache (or the other way around)
> your code adds port_state_cache to the mix but with the same issue.
> I guess that should be fixed.
>
Hi Mark,
Not really. The release was done by ib_dealloc_device() when put the
last ref of kobj,
dev_release -> ib_device_release -> ib_cache_release_one, driver was supposed
to use it to destroy the device when register failed.
Cheers,
Jinpu
>> !device->cache.lmc_cache) {
>> pr_warn("Couldn't allocate cache for %s\n", device->name);
>> return -ENOMEM;
>> @@ -1213,6 +1219,7 @@ void ib_cache_release_one(struct ib_device *device)
>> gid_table_release_one(device);
>> kfree(device->cache.pkey_cache);
>> kfree(device->cache.lmc_cache);
>> + kfree(device->cache.port_state_cache);
>> }
>>
>> void ib_cache_cleanup_one(struct ib_device *device)
>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>> index 5ad43a4..a167ae0 100644
>> --- a/include/rdma/ib_verbs.h
>> +++ b/include/rdma/ib_verbs.h
>> @@ -1761,6 +1761,7 @@ struct ib_cache {
>> struct ib_pkey_cache **pkey_cache;
>> struct ib_gid_table **gid_cache;
>> u8 *lmc_cache;
>> + enum ib_port_state *port_state_cache;
>> };
>>
>> struct ib_dma_mapping_ops {
>>
>
> Mark.
--
Jinpu Wang
Linux Kernel Developer
ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin
Tel: +49 30 577 008 042
Fax: +49 30 577 008 299
Email: jinpu.wang@xxxxxxxxxxxxxxxx
URL: https://www.profitbricks.de
Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
