From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hannes Reinecke Subject: Re: [PATCH 10/17] scsi_dh_alua: Use separate alua_port_group structure Date: Thu, 07 May 2015 15:46:59 +0200 Message-ID: <554B6CD3.5070000@suse.de> References: <1430743343-47174-1-git-send-email-hare@suse.de> <1430743343-47174-11-git-send-email-hare@suse.de> <554B5BD8.2090605@sandisk.com> <554B6A48.1090405@sandisk.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from cantor2.suse.de ([195.135.220.15]:47079 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751072AbbEGNrB (ORCPT ); Thu, 7 May 2015 09:47:01 -0400 In-Reply-To: <554B6A48.1090405@sandisk.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Bart Van Assche , James Bottomley Cc: Christoph Hellwig , linux-scsi@vger.kernel.org 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 =3D 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 =3D group_id; >>> + pg->buff =3D pg->inq; >>> + pg->bufflen =3D ALUA_INQUIRY_SIZE; >>> + pg->tpgs =3D h->tpgs; >>> + pg->state =3D TPGS_STATE_OPTIMIZED; >>> + kref_init(&pg->kref); >>> + list_add(&pg->node, &port_group_list); >>> + h->pg =3D 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. >=20 > (replying to my own e-mail) >=20 > Hello Hannes, >=20 > 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 ? >=20 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 --=20 Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N=FCrnberg GF: F. Imend=F6rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG N=FCrnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html