From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6EA3920E006 for ; Tue, 18 Mar 2025 14:10:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=195.135.223.130 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742307036; cv=none; b=tGs49oSPqln4tqz9WPQxFEQWj/ryWcqyLcI4qAi9O4sEJYVsvHkPKi6v5mARe02Q24RQuxYT3JzrGa8/E4ceRUwt57tkcJaPpkFeWx3ZigLgqe55fuhQ+HwAvi9gsIjMhfNely0fYaL02KabfEjvA6S9l4JmD6H/jjYEf1KJ4xk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742307036; c=relaxed/simple; bh=Hh7+4H2OqzHvehRHdsNqC5RBGFRkij8niL4MoJc4CC0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=U82FDt9PDgtDXiuqk0VcFuGphgskAT6yjYYAvnVxsiqYWGaZ8Md9XHWxfFDjYQpkCfecDr0P82RDsGtUy7+alWWTYFecDI1u9Qy5z9k614suPxkZcfBXzRVsrTrPj3hHrX6WksnBZXHQzOuhNqlimpLfZ3hNYB+ES3nKmuGtYeE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=suse.de; spf=pass smtp.mailfrom=suse.de; arc=none smtp.client-ip=195.135.223.130 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.de Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id B0B1821940; Tue, 18 Mar 2025 14:10:29 +0000 (UTC) Authentication-Results: smtp-out1.suse.de; none Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 87E1E1379A; Tue, 18 Mar 2025 14:10:29 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id SzbNH9V+2WcJBgAAD6G6ig (envelope-from ); Tue, 18 Mar 2025 14:10:29 +0000 Message-ID: Date: Tue, 18 Mar 2025 15:10:29 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 04/18] nvmet-fcloop: refactor fcloop_nport_alloc To: Daniel Wagner Cc: Daniel Wagner , James Smart , Christoph Hellwig , Sagi Grimberg , Chaitanya Kulkarni , Keith Busch , linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org References: <20250318-nvmet-fcloop-v3-0-05fec0fc02f6@kernel.org> <20250318-nvmet-fcloop-v3-4-05fec0fc02f6@kernel.org> <66261f08-5386-4b22-aa6f-7be1d4023fee@flourine.local> Content-Language: en-US From: Hannes Reinecke In-Reply-To: <66261f08-5386-4b22-aa6f-7be1d4023fee@flourine.local> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spam-Level: X-Spamd-Result: default: False [-4.00 / 50.00]; REPLY(-4.00)[] X-Spam-Score: -4.00 X-Spam-Flag: NO X-Rspamd-Queue-Id: B0B1821940 X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Rspamd-Action: no action X-Rspamd-Server: rspamd2.dmz-prg2.suse.org On 3/18/25 14:38, Daniel Wagner wrote: > On Tue, Mar 18, 2025 at 12:02:48PM +0100, Hannes Reinecke wrote: >>> - list_for_each_entry(tmplport, &fcloop_lports, lport_list) { >>> - if (tmplport->localport->node_name == opts->wwnn && >>> - tmplport->localport->port_name == opts->wwpn) >>> - goto out_invalid_opts; >>> + INIT_LIST_HEAD(&nport->nport_list); >>> + nport->node_name = opts->wwnn; >>> + nport->port_name = opts->wwpn; >>> + refcount_set(&nport->ref, 1); >>> - if (tmplport->localport->node_name == opts->lpwwnn && >>> - tmplport->localport->port_name == opts->lpwwpn) >>> - lport = tmplport; >>> + spin_lock_irqsave(&fcloop_lock, flags); >>> + list_add_tail(&nport->nport_list, &fcloop_nports); >>> + spin_unlock_irqrestore(&fcloop_lock, flags); >>> } >> Hmm. I don't really like this pattern; there is a race condition >> between lookup and allocation leading to possibly duplicate entries >> on the list. > > Yes, that's not a good thing. > >> Lookup and allocation really need to be under the same lock. > > This means the new entry has always to be allocated first and then we > either free it again or insert into the list, because it's not possible > to allocate under the spinlock. Not that beautiful but correctness wins. Allocate first, and then free it if the entry is already present. Slightly wasteful, but that's what it is. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich