Re: [PATCH 10/17] scsi_dh_alua: Use separate alua_port_group structure

[Hannes Reinecke <hare@suse.de>]

On 05/07/2015 02:34 PM, Bart Van Assche wrote:
> On 05/04/15 14:42, 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);
> 
> Does this really need to be a warning ?
> 
No, you are right. This it mainly for debugging.

>> +    spin_lock(&port_group_lock);
>> +    pg = kzalloc(sizeof(struct alua_port_group), GFP_ATOMIC);
>> +    if (!pg) {
>> +        sdev_printk(KERN_WARNING, sdev,
>> +                "%s: kzalloc port group failed\n",
>> +                ALUA_DH_NAME);
>> +        /* Temporary failure, bypass */
>> +        spin_unlock(&port_group_lock);
>> +        return SCSI_DH_DEV_TEMP_BUSY;
>> +    }
>> +    pg->group_id = group_id;
>> +    pg->buff = pg->inq;
>> +    pg->bufflen = ALUA_INQUIRY_SIZE;
>> +    pg->tpgs = h->tpgs;
>> +    pg->state = TPGS_STATE_OPTIMIZED;
>> +    kref_init(&pg->kref);
>> +    list_add(&pg->node, &port_group_list);
>> +    h->pg = pg;
>> +    spin_unlock(&port_group_lock);
> 
> Sorry but it's not clear to me why the kzalloc() statement happens
> under port_group_lock. Please consider to perform only the
> list_add() statement under port_group_lock, to perform all other
> assignments without holding that lock and to change GFP_ATOMIC into
> GFP_KERNEL.
> 
Right, kzalloc() doesn't need to happen under the lock.

>> -    len = (h->buff[0] << 24) + (h->buff[1] << 16) +
>> -        (h->buff[2] << 8) + h->buff[3] + 4;
>> +    len = (pg->buff[0] << 24) + (pg->buff[1] << 16) +
>> +        (pg->buff[2] << 8) + pg->buff[3] + 4;
> 
> Has it been considered to use get_unaligned_be32() instead ?
> 
Okay, will do.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html