On 03/07/2014 02:41 AM, Christoph Hellwig wrote:
- se_tpg->tpg_lun_list = array_zalloc(TRANSPORT_MAX_LUNS_PER_TPG,
- sizeof(struct se_lun), GFP_KERNEL);
+ se_tpg->tpg_lun_list = kzalloc(TRANSPORT_MAX_LUNS_PER_TPG * sizeof(void*),
+ GFP_KERNEL);
Again seems like the pointer array should just be embedded instead of
dynamically allocated. Also given that it's properly typdef the kcalloc
should have used the type instead of void *. And it should have been
kcalloc to start with..
Yup. I posted a "7 of 6" patch that folds the arrays into their parent
structures.
+void core_tpg_free_lun(
+ struct se_portal_group *tpg,
+ struct se_lun *lun)
+{
spin_lock(&tpg->tpg_lun_lock);
- lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
+ tpg->tpg_lun_list[lun->unpacked_lun] = NULL;
spin_unlock(&tpg->tpg_lun_lock);
+
+ kfree(lun);
I can't see how the synchronization can work without refcounting the lun
structure. The lock just protectes the assignment, but you free it
right after. What happens to how jsut dereferenced it under the lock
but then uses it outside (e.g. core_dev_add_initiator_node_lun_acl).
Well you're right, but this is one instance of a larger lio
locking/refcounting hairball. This will be addressed in a separate patch
series.
Regards -- Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
