On 05/07/2015 03:36 PM, Bart Van Assche wrote:
> 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 ?
>
The lock is only taken for list modifications; the actual
accesses are done via RCU references.
So converting this to a mutex would give an insignificant
advantage. Plus I'm not sure if using a mutex in the face
for RCU accesses is valid; the documentation only uses
spinlocks here.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
