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

Bart Van Assche
2015-05-07T13:36:14Z

On 05/07/15 14:34, Bart Van Assche wrote:
> On 05/04/15 14:42, Hannes Reinecke wrote:
>> + 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.

(replying to my own e-mail)

Hello Hannes,

Is it correct that port_group_list is only accessed from thread context
? If so, has it been considered to change port_group_lock into a mutex
instead ?

Thanks,

Bart.