On 01/31/14 10:29, Hannes Reinecke wrote:
> +static void release_port_group(struct kref *kref)
> +{
> + struct alua_port_group *pg;
> +
> + pg = container_of(kref, struct alua_port_group, kref);
> + printk(KERN_WARNING "alua: release port group %d\n", pg->group_id);
> + spin_lock(&port_group_lock);
> + list_del(&pg->node);
> + spin_unlock(&port_group_lock);
> + if (pg->buff && pg->inq != pg->buff)
> + kfree(pg->buff);
> + kfree(pg);
> +}
Does that message really need level "WARNING" ?
> + sdev_printk(KERN_INFO, sdev,
> + "%s: port group %02x rel port %02x\n",
> + ALUA_DH_NAME, group_id, h->rel_port);
> + spin_lock(&port_group_lock);
> + pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
> + if (!pg) {
> + sdev_printk(KERN_WARNING, sdev,
> + "%s: kzalloc port group failed\n",
> + ALUA_DH_NAME);
> + /* Temporary failure, bypass */
> + spin_unlock(&port_group_lock);
> + err = SCSI_DH_DEV_TEMP_BUSY;
> + goto out;
> + }
> + pg->group_id = group_id;
> + pg->buff = pg->inq;
> + pg->bufflen = ALUA_INQUIRY_SIZE;
> + pg->tpgs = h->tpgs;
> + pg->state = TPGS_STATE_OPTIMIZED;
> + pg->flags = h->flags;
> + kref_init(&pg->kref);
> + list_add(&pg->node, &port_group_list);
> + h->pg = pg;
> + spin_unlock(&port_group_lock);
> + err = SCSI_DH_OK;
A GFP_KERNEL allocation with a spin lock held ?? Please move that
allocation outside the critical section and also the code for
initializing *pg. What's not clear to me is why no uniqueness check is
performed before invoking list_add() ? Does that mean that information
for the same port group ID can end up multiple times in the
port_group_list ? Such a uniqueness check can only be performed if a
storage array identification is available so patches 07 and 08 may have
to be swapped.
Bart.
--
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
