Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1304251

Re: [PATCH 1/4] target: Obtain se_node_acl->acl_kref during get_initiator_node_acl

From Christoph Hellwig <hch@lst.de>
Newsgroups linux.kernel
Subject Re: [PATCH 1/4] target: Obtain se_node_acl->acl_kref during get_initiator_node_acl
Date 2016-01-08 09:20 +0100
Message-ID <qOAr7-413-1@gated-at.bofh.it> (permalink)
References <qOzv3-3m3-11@gated-at.bofh.it> <qOzv3-3m3-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


>  	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.

While we're at it - is there any god reason to keep acl_pr_ref_count
as a separate entity from acl_kref?

>  void transport_free_session(struct se_session *se_sess)
>  {
> +	struct se_node_acl *se_nacl = se_sess->se_node_acl;
> +	/*
> +	 * Drop the se_node_acl->nacl_kref obtained from within
> +	 * core_tpg_get_initiator_node_acl().
> +	 */
> +	if (se_nacl) {
> +		se_sess->se_node_acl = NULL;

Whats the point of zeroing se_node_acl just before freeing se_sess?

>  	/*
>  	 * If last kref is dropping now for an explicit NodeACL, awake sleeping
>  	 * ->acl_free_comp caller to wakeup configfs se_node_acl->acl_group
> -	 * removal context.
> +	 * removal context from within transport_free_session() code.
>  	 */

The comment neds to move to transport_free_session.  Or maybe just removed
given that it's obvious from the code flow.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/4] target: Close se_node_acl lookup race "Nicholas A. Bellinger" <nab@daterainc.com> - 2016-01-08 08:20 +0100
  [PATCH 1/4] target: Obtain se_node_acl->acl_kref during get_initiator_node_acl "Nicholas A. Bellinger" <nab@daterainc.com> - 2016-01-08 08:20 +0100
    Re: [PATCH 1/4] target: Obtain se_node_acl->acl_kref during  get_initiator_node_acl Christoph Hellwig <hch@lst.de> - 2016-01-08 09:20 +0100
      Re: [PATCH 1/4] target: Obtain se_node_acl->acl_kref during  get_initiator_node_acl Christoph Hellwig <hch@lst.de> - 2016-01-08 09:40 +0100
      Re: [PATCH 1/4] target: Obtain se_node_acl->acl_kref during  get_initiator_node_acl "Nicholas A. Bellinger" <nab@linux-iscsi.org> - 2016-01-08 09:50 +0100
        Re: [PATCH 1/4] target: Obtain se_node_acl->acl_kref during  get_initiator_node_acl Christoph Hellwig <hch@lst.de> - 2016-01-08 10:10 +0100
          Re: [PATCH 1/4] target: Obtain se_node_acl->acl_kref during  get_initiator_node_acl Christoph Hellwig <hch@lst.de> - 2016-01-08 10:40 +0100
  [PATCH 2/4] target: Remove useless set_initiator_node_queue_depth acl lookup "Nicholas A. Bellinger" <nab@daterainc.com> - 2016-01-08 08:20 +0100
    Re: [PATCH 2/4] target: Remove useless  set_initiator_node_queue_depth acl lookup Christoph Hellwig <hch@lst.de> - 2016-01-08 09:20 +0100

csiph-web