From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754431AbcAHIro (ORCPT <rfc822;w@1wt.eu>);
	Fri, 8 Jan 2016 03:47:44 -0500
Received: from mail.linux-iscsi.org ([67.23.28.174]:33006 "EHLO
	linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754396AbcAHIrl (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 8 Jan 2016 03:47:41 -0500
Message-ID: <1452242859.27508.12.camel@haakon3.risingtidesystems.com>
Subject: Re: [PATCH 1/4] target: Obtain se_node_acl->acl_kref during
 get_initiator_node_acl
From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>,
        "Nicholas A. Bellinger" <nab@daterainc.com>,
        target-devel <target-devel@vger.kernel.org>,
        linux-scsi <linux-scsi@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        Sagi Grimberg <sagig@mellanox.com>, Hannes Reinecke <hare@suse.de>,
        Andy Grover <agrover@redhat.com>, Vasu Dev <vasu.dev@linux.intel.com>,
        Vu Pham <vu@mellanox.com>
Date: Fri, 08 Jan 2016 00:47:39 -0800
In-Reply-To: <568F73D2.2090808@sandisk.com>
References: <1452237348-2277-1-git-send-email-nab@daterainc.com>
	 <1452237348-2277-2-git-send-email-nab@daterainc.com>
	 <20160108081412.GA32138@lst.de> <568F73D2.2090808@sandisk.com>
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.4.4-1 
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, 2016-01-08 at 09:31 +0100, Bart Van Assche wrote:
> On 01/08/2016 09:14 AM, Christoph Hellwig wrote:
> >>   	mutex_lock(&tpg->acl_node_mutex);
> >>   	acl = __core_tpg_get_initiator_node_acl(tpg, initiatorname);
> >> +	/*
> >> +	 * Obtain the acl_kref now, which will be dropped upon the
> >> +	 * release of se_sess memory within transport_free_session().
> >> +	 */
> >> +	if (acl)
> >> +		kref_get(&acl->acl_kref);
> >
> > І think the comment is highly confusing as it's about one of the
> > callers, while the function has many.
> >
> > I'd suggest you move it to core_tpg_check_initiator_node_acl instead.
> >
> > Also I think iscsit_build_sendtargets_response will need a put on
> > the nacl, otherwise you'll leak references.
> 
> Indeed. All error paths in all target drivers will have to be modified 
> to avoid that an acl reference leak is triggered if 
> transport_init_session() fails after core_tpg_check_initiator_node_acl() 
> succeeded.
> 

Actually no, they do not.  That's the way that everything outside of
tcm_fc + ib_srpt driver code has already worked for a long time.