Re: [PATCH 06/32] target: Convert lu_gp_ref_cnt to kref

2014-02-07 Thread Nicholas A. Bellinger
On Thu, 2014-02-06 at 18:56 -0800, Andy Grover wrote:
> On 02/06/2014 03:51 PM, Nicholas A. Bellinger wrote:
> > The problem with this patch and all of the other patches that follow the
> > same logic is the false assumption that it's safe to return from
> > configfs_group_operations->drop_item() before all references to the
> > underlying data structure containing the associated struct config_group
> > have been dropped.
> > 
> > In this particular case, target_core_alua_drop_lu_gp() ->
> > config_item_put() -> target_core_alua_lu_gp_release() ->
> > core_alua_free_lu_gp() is still being called from ->drop_item() via
> > target_core_alua_lu_gp_ops->release(), so the same holds true here as
> > well.
> 
> Yes exactly. That's why the configfs release() doesn't free the lu_gp,
> it just lowers the lu_gp refcount. release() being called doesn't mean
> the object is going away, it just means configfs is done with it.
> 
> If do_port_transition has incremented it in the meantime, the lu_gp
> won't be freed from the release() (because the underlying object's
> refcount will still be nonzero) but only when do_port_transition is
> done. In the normal case where there isn't a ref from
> do_port_transition, then it is safe to free the lu_gp from release ->
> put_alua_lu_gp -> release_alua_lu_gp -> kmem_cache_free.

It's still completely wrong to return from ->drop_item() before all
references to the associated struct config_group have dropped because
nothing prevents a parent struct config_group (and it's parent above
that) from also being removed.

The whole point is that this type of parent/child reference counting
comes for free with configfs, and patches that introduce changes that
don't actually wait for individual references to drop, but instead
lazily allow references to drop in the background at their leisure break
this model.

--nab

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/32] target: Convert lu_gp_ref_cnt to kref

2014-02-06 Thread Andy Grover
On 02/06/2014 03:51 PM, Nicholas A. Bellinger wrote:
> The problem with this patch and all of the other patches that follow the
> same logic is the false assumption that it's safe to return from
> configfs_group_operations->drop_item() before all references to the
> underlying data structure containing the associated struct config_group
> have been dropped.
> 
> In this particular case, target_core_alua_drop_lu_gp() ->
> config_item_put() -> target_core_alua_lu_gp_release() ->
> core_alua_free_lu_gp() is still being called from ->drop_item() via
> target_core_alua_lu_gp_ops->release(), so the same holds true here as
> well.

Yes exactly. That's why the configfs release() doesn't free the lu_gp,
it just lowers the lu_gp refcount. release() being called doesn't mean
the object is going away, it just means configfs is done with it.

If do_port_transition has incremented it in the meantime, the lu_gp
won't be freed from the release() (because the underlying object's
refcount will still be nonzero) but only when do_port_transition is
done. In the normal case where there isn't a ref from
do_port_transition, then it is safe to free the lu_gp from release ->
put_alua_lu_gp -> release_alua_lu_gp -> kmem_cache_free.

-- Andy

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/32] target: Convert lu_gp_ref_cnt to kref

2014-02-06 Thread Nicholas A. Bellinger
On Wed, 2014-02-05 at 14:02 -0800, Andy Grover wrote:
> Hi nab, I'm getting back to looking at this patchset, but wanted to just
> discuss and understand this one first because all the kref ones are
> similar. see below.
> 
> On 12/16/2013 12:52 PM, Nicholas A. Bellinger wrote:
> > On Fri, 2013-12-13 at 15:58 -0800, Andy Grover wrote:
> >> Use kref to handle reference counting
> >>
> >> Signed-off-by: Andy Grover 
> >> ---
> >>  drivers/target/target_core_alua.c |   37 
> >> -
> >>  include/target/target_core_base.h |2 +-
> >>  2 files changed, 21 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/target/target_core_alua.c 
> >> b/drivers/target/target_core_alua.c
> >> index 2ac2f11..8c01ade 100644
> >> --- a/drivers/target/target_core_alua.c
> >> +++ b/drivers/target/target_core_alua.c
> >> @@ -54,6 +54,16 @@ static LIST_HEAD(lu_gps_list);
> >>  
> >>  struct t10_alua_lu_gp *default_lu_gp;
> >>  
> >> +static void release_alua_lu_gp(struct kref *ref)
> >> +{
> >> +  struct t10_alua_lu_gp *lu_gp = container_of(ref, struct t10_alua_lu_gp, 
> >> refcount);
> >> +
> >> +  kmem_cache_free(t10_alua_lu_gp_cache, lu_gp);
> >> +}
> >> +
> >> +#define get_alua_lu_gp(x) kref_get(&x->refcount)
> >> +#define put_alua_lu_gp(x) kref_put(&x->refcount, release_alua_lu_gp)
> >> +
> >>  /*
> >>   * REPORT_TARGET_PORT_GROUPS
> >>   *
> >> @@ -898,8 +908,7 @@ int core_alua_do_port_transition(
> >>local_lu_gp_mem = l_dev->dev_alua_lu_gp_mem;
> >>spin_lock(&local_lu_gp_mem->lu_gp_mem_lock);
> >>lu_gp = local_lu_gp_mem->lu_gp;
> >> -  atomic_inc(&lu_gp->lu_gp_ref_cnt);
> >> -  smp_mb__after_atomic_inc();
> >> +  get_alua_lu_gp(lu_gp);
> >>spin_unlock(&local_lu_gp_mem->lu_gp_mem_lock);
> >>/*
> >> * For storage objects that are members of the 'default_lu_gp',
> >> @@ -913,8 +922,8 @@ int core_alua_do_port_transition(
> >> */
> >>core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
> >>md_buf, new_state, explicit);
> >> -  atomic_dec(&lu_gp->lu_gp_ref_cnt);
> >> -  smp_mb__after_atomic_dec();
> >> +
> >> +  put_alua_lu_gp(lu_gp);
> >>kfree(md_buf);
> >>return 0;
> >>}
> >> @@ -985,8 +994,7 @@ int core_alua_do_port_transition(
> >>l_tg_pt_gp->tg_pt_gp_id, (explicit) ? "explicit" : "implicit",
> >>core_alua_dump_state(new_state));
> >>  
> >> -  atomic_dec(&lu_gp->lu_gp_ref_cnt);
> >> -  smp_mb__after_atomic_dec();
> >> +  put_alua_lu_gp(lu_gp);
> >>kfree(md_buf);
> >>return 0;
> >>  }
> >> @@ -1107,7 +1115,8 @@ core_alua_allocate_lu_gp(const char *name, int 
> >> def_group)
> >>INIT_LIST_HEAD(&lu_gp->lu_gp_node);
> >>INIT_LIST_HEAD(&lu_gp->lu_gp_mem_list);
> >>spin_lock_init(&lu_gp->lu_gp_lock);
> >> -  atomic_set(&lu_gp->lu_gp_ref_cnt, 0);
> >> +
> >> +  kref_init(&lu_gp->refcount);
> >>  
> >>if (def_group) {
> >>lu_gp->lu_gp_id = alua_lu_gps_counter++;
> >> @@ -1200,13 +1209,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp 
> >> *lu_gp)
> >>list_del(&lu_gp->lu_gp_node);
> >>alua_lu_gps_count--;
> >>spin_unlock(&lu_gps_lock);
> >> -  /*
> >> -   * Allow struct t10_alua_lu_gp * referenced by 
> >> core_alua_get_lu_gp_by_name()
> >> -   * in target_core_configfs.c:target_core_store_alua_lu_gp() to be
> >> -   * released with core_alua_put_lu_gp_from_name()
> >> -   */
> >> -  while (atomic_read(&lu_gp->lu_gp_ref_cnt))
> >> -  cpu_relax();
> >> +
> >>/*
> >> * Release reference to struct t10_alua_lu_gp * from all associated
> >> * struct se_device.
> >> @@ -1241,7 +1244,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp 
> >> *lu_gp)
> >>}
> >>spin_unlock(&lu_gp->lu_gp_lock);
> >>  
> >> -  kmem_cache_free(t10_alua_lu_gp_cache, lu_gp);
> >> +  put_alua_lu_gp(lu_gp);
> >>  }
> >>  
> 
> > The assumption that it's safe to 'Release reference to struct
> > t10_alua_lu_gp * from all associated struct device' below the original
> > cpu_relax(), while there are still other process contexts doing their
> > respective put_alua_lu_gp() is totally wrong.
> 
> The only other spot is core_alua_do_port_transition, afaics. I think if
> it races with free_lu_gp, lu_gp will either be the old lu_gp (which no
> longer will have anything on lu_gp_mem_list) or will be default_lu_gp.
> If it's the old lu_gp then it iterates over an empty list, and then the
> lu_gp gets finally freed by the put() at the bottom.
> 
> > Furthermore, allowing a configfs_group_ops->drop_item() to return while
> > there are still active references from other process contexts means that
> > the parent struct config_group is no longer referenced counted (eg:
> > configfs child is removed), and introduces a whole host of potential
> > bugs.
> > 
> > So that said, NAK on this patch.
> 
> I think some of the other patches used drop_item() and thus were bad,
> but in this one the existing c

Re: [PATCH 06/32] target: Convert lu_gp_ref_cnt to kref

2014-02-05 Thread Andy Grover
Hi nab, I'm getting back to looking at this patchset, but wanted to just
discuss and understand this one first because all the kref ones are
similar. see below.

On 12/16/2013 12:52 PM, Nicholas A. Bellinger wrote:
> On Fri, 2013-12-13 at 15:58 -0800, Andy Grover wrote:
>> Use kref to handle reference counting
>>
>> Signed-off-by: Andy Grover 
>> ---
>>  drivers/target/target_core_alua.c |   37 
>> -
>>  include/target/target_core_base.h |2 +-
>>  2 files changed, 21 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/target/target_core_alua.c 
>> b/drivers/target/target_core_alua.c
>> index 2ac2f11..8c01ade 100644
>> --- a/drivers/target/target_core_alua.c
>> +++ b/drivers/target/target_core_alua.c
>> @@ -54,6 +54,16 @@ static LIST_HEAD(lu_gps_list);
>>  
>>  struct t10_alua_lu_gp *default_lu_gp;
>>  
>> +static void release_alua_lu_gp(struct kref *ref)
>> +{
>> +struct t10_alua_lu_gp *lu_gp = container_of(ref, struct t10_alua_lu_gp, 
>> refcount);
>> +
>> +kmem_cache_free(t10_alua_lu_gp_cache, lu_gp);
>> +}
>> +
>> +#define get_alua_lu_gp(x) kref_get(&x->refcount)
>> +#define put_alua_lu_gp(x) kref_put(&x->refcount, release_alua_lu_gp)
>> +
>>  /*
>>   * REPORT_TARGET_PORT_GROUPS
>>   *
>> @@ -898,8 +908,7 @@ int core_alua_do_port_transition(
>>  local_lu_gp_mem = l_dev->dev_alua_lu_gp_mem;
>>  spin_lock(&local_lu_gp_mem->lu_gp_mem_lock);
>>  lu_gp = local_lu_gp_mem->lu_gp;
>> -atomic_inc(&lu_gp->lu_gp_ref_cnt);
>> -smp_mb__after_atomic_inc();
>> +get_alua_lu_gp(lu_gp);
>>  spin_unlock(&local_lu_gp_mem->lu_gp_mem_lock);
>>  /*
>>   * For storage objects that are members of the 'default_lu_gp',
>> @@ -913,8 +922,8 @@ int core_alua_do_port_transition(
>>   */
>>  core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
>>  md_buf, new_state, explicit);
>> -atomic_dec(&lu_gp->lu_gp_ref_cnt);
>> -smp_mb__after_atomic_dec();
>> +
>> +put_alua_lu_gp(lu_gp);
>>  kfree(md_buf);
>>  return 0;
>>  }
>> @@ -985,8 +994,7 @@ int core_alua_do_port_transition(
>>  l_tg_pt_gp->tg_pt_gp_id, (explicit) ? "explicit" : "implicit",
>>  core_alua_dump_state(new_state));
>>  
>> -atomic_dec(&lu_gp->lu_gp_ref_cnt);
>> -smp_mb__after_atomic_dec();
>> +put_alua_lu_gp(lu_gp);
>>  kfree(md_buf);
>>  return 0;
>>  }
>> @@ -1107,7 +1115,8 @@ core_alua_allocate_lu_gp(const char *name, int 
>> def_group)
>>  INIT_LIST_HEAD(&lu_gp->lu_gp_node);
>>  INIT_LIST_HEAD(&lu_gp->lu_gp_mem_list);
>>  spin_lock_init(&lu_gp->lu_gp_lock);
>> -atomic_set(&lu_gp->lu_gp_ref_cnt, 0);
>> +
>> +kref_init(&lu_gp->refcount);
>>  
>>  if (def_group) {
>>  lu_gp->lu_gp_id = alua_lu_gps_counter++;
>> @@ -1200,13 +1209,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp 
>> *lu_gp)
>>  list_del(&lu_gp->lu_gp_node);
>>  alua_lu_gps_count--;
>>  spin_unlock(&lu_gps_lock);
>> -/*
>> - * Allow struct t10_alua_lu_gp * referenced by 
>> core_alua_get_lu_gp_by_name()
>> - * in target_core_configfs.c:target_core_store_alua_lu_gp() to be
>> - * released with core_alua_put_lu_gp_from_name()
>> - */
>> -while (atomic_read(&lu_gp->lu_gp_ref_cnt))
>> -cpu_relax();
>> +
>>  /*
>>   * Release reference to struct t10_alua_lu_gp * from all associated
>>   * struct se_device.
>> @@ -1241,7 +1244,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp *lu_gp)
>>  }
>>  spin_unlock(&lu_gp->lu_gp_lock);
>>  
>> -kmem_cache_free(t10_alua_lu_gp_cache, lu_gp);
>> +put_alua_lu_gp(lu_gp);
>>  }
>>  

> The assumption that it's safe to 'Release reference to struct
> t10_alua_lu_gp * from all associated struct device' below the original
> cpu_relax(), while there are still other process contexts doing their
> respective put_alua_lu_gp() is totally wrong.

The only other spot is core_alua_do_port_transition, afaics. I think if
it races with free_lu_gp, lu_gp will either be the old lu_gp (which no
longer will have anything on lu_gp_mem_list) or will be default_lu_gp.
If it's the old lu_gp then it iterates over an empty list, and then the
lu_gp gets finally freed by the put() at the bottom.

> Furthermore, allowing a configfs_group_ops->drop_item() to return while
> there are still active references from other process contexts means that
> the parent struct config_group is no longer referenced counted (eg:
> configfs child is removed), and introduces a whole host of potential
> bugs.
> 
> So that said, NAK on this patch.

I think some of the other patches used drop_item() and thus were bad,
but in this one the existing code is already calling
core_alua_free_lu_gp() from release().

Thoughts?

Thanks in advance -- Regards -- Andy

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to

Re: [PATCH 06/32] target: Convert lu_gp_ref_cnt to kref

2013-12-16 Thread Nicholas A. Bellinger
On Fri, 2013-12-13 at 15:58 -0800, Andy Grover wrote:
> Use kref to handle reference counting
> 
> Signed-off-by: Andy Grover 
> ---
>  drivers/target/target_core_alua.c |   37 
> -
>  include/target/target_core_base.h |2 +-
>  2 files changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/target/target_core_alua.c 
> b/drivers/target/target_core_alua.c
> index 2ac2f11..8c01ade 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -54,6 +54,16 @@ static LIST_HEAD(lu_gps_list);
>  
>  struct t10_alua_lu_gp *default_lu_gp;
>  
> +static void release_alua_lu_gp(struct kref *ref)
> +{
> + struct t10_alua_lu_gp *lu_gp = container_of(ref, struct t10_alua_lu_gp, 
> refcount);
> +
> + kmem_cache_free(t10_alua_lu_gp_cache, lu_gp);
> +}
> +
> +#define get_alua_lu_gp(x) kref_get(&x->refcount)
> +#define put_alua_lu_gp(x) kref_put(&x->refcount, release_alua_lu_gp)
> +
>  /*
>   * REPORT_TARGET_PORT_GROUPS
>   *
> @@ -898,8 +908,7 @@ int core_alua_do_port_transition(
>   local_lu_gp_mem = l_dev->dev_alua_lu_gp_mem;
>   spin_lock(&local_lu_gp_mem->lu_gp_mem_lock);
>   lu_gp = local_lu_gp_mem->lu_gp;
> - atomic_inc(&lu_gp->lu_gp_ref_cnt);
> - smp_mb__after_atomic_inc();
> + get_alua_lu_gp(lu_gp);
>   spin_unlock(&local_lu_gp_mem->lu_gp_mem_lock);
>   /*
>* For storage objects that are members of the 'default_lu_gp',
> @@ -913,8 +922,8 @@ int core_alua_do_port_transition(
>*/
>   core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
>   md_buf, new_state, explicit);
> - atomic_dec(&lu_gp->lu_gp_ref_cnt);
> - smp_mb__after_atomic_dec();
> +
> + put_alua_lu_gp(lu_gp);
>   kfree(md_buf);
>   return 0;
>   }
> @@ -985,8 +994,7 @@ int core_alua_do_port_transition(
>   l_tg_pt_gp->tg_pt_gp_id, (explicit) ? "explicit" : "implicit",
>   core_alua_dump_state(new_state));
>  
> - atomic_dec(&lu_gp->lu_gp_ref_cnt);
> - smp_mb__after_atomic_dec();
> + put_alua_lu_gp(lu_gp);
>   kfree(md_buf);
>   return 0;
>  }
> @@ -1107,7 +1115,8 @@ core_alua_allocate_lu_gp(const char *name, int 
> def_group)
>   INIT_LIST_HEAD(&lu_gp->lu_gp_node);
>   INIT_LIST_HEAD(&lu_gp->lu_gp_mem_list);
>   spin_lock_init(&lu_gp->lu_gp_lock);
> - atomic_set(&lu_gp->lu_gp_ref_cnt, 0);
> +
> + kref_init(&lu_gp->refcount);
>  
>   if (def_group) {
>   lu_gp->lu_gp_id = alua_lu_gps_counter++;
> @@ -1200,13 +1209,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp *lu_gp)
>   list_del(&lu_gp->lu_gp_node);
>   alua_lu_gps_count--;
>   spin_unlock(&lu_gps_lock);
> - /*
> -  * Allow struct t10_alua_lu_gp * referenced by 
> core_alua_get_lu_gp_by_name()
> -  * in target_core_configfs.c:target_core_store_alua_lu_gp() to be
> -  * released with core_alua_put_lu_gp_from_name()
> -  */
> - while (atomic_read(&lu_gp->lu_gp_ref_cnt))
> - cpu_relax();
> +
>   /*
>* Release reference to struct t10_alua_lu_gp * from all associated
>* struct se_device.
> @@ -1241,7 +1244,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp *lu_gp)
>   }
>   spin_unlock(&lu_gp->lu_gp_lock);
>  
> - kmem_cache_free(t10_alua_lu_gp_cache, lu_gp);
> + put_alua_lu_gp(lu_gp);
>  }
>  

The assumption that it's safe to 'Release reference to struct
t10_alua_lu_gp * from all associated struct device' below the original
cpu_relax(), while there are still other process contexts doing their
respective put_alua_lu_gp() is totally wrong.

Furthermore, allowing a configfs_group_ops->drop_item() to return while
there are still active references from other process contexts means that
the parent struct config_group is no longer referenced counted (eg:
configfs child is removed), and introduces a whole host of potential
bugs.

So that said, NAK on this patch.

--nab

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 06/32] target: Convert lu_gp_ref_cnt to kref

2013-12-13 Thread Andy Grover
Use kref to handle reference counting

Signed-off-by: Andy Grover 
---
 drivers/target/target_core_alua.c |   37 -
 include/target/target_core_base.h |2 +-
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/target/target_core_alua.c 
b/drivers/target/target_core_alua.c
index 2ac2f11..8c01ade 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -54,6 +54,16 @@ static LIST_HEAD(lu_gps_list);
 
 struct t10_alua_lu_gp *default_lu_gp;
 
+static void release_alua_lu_gp(struct kref *ref)
+{
+   struct t10_alua_lu_gp *lu_gp = container_of(ref, struct t10_alua_lu_gp, 
refcount);
+
+   kmem_cache_free(t10_alua_lu_gp_cache, lu_gp);
+}
+
+#define get_alua_lu_gp(x) kref_get(&x->refcount)
+#define put_alua_lu_gp(x) kref_put(&x->refcount, release_alua_lu_gp)
+
 /*
  * REPORT_TARGET_PORT_GROUPS
  *
@@ -898,8 +908,7 @@ int core_alua_do_port_transition(
local_lu_gp_mem = l_dev->dev_alua_lu_gp_mem;
spin_lock(&local_lu_gp_mem->lu_gp_mem_lock);
lu_gp = local_lu_gp_mem->lu_gp;
-   atomic_inc(&lu_gp->lu_gp_ref_cnt);
-   smp_mb__after_atomic_inc();
+   get_alua_lu_gp(lu_gp);
spin_unlock(&local_lu_gp_mem->lu_gp_mem_lock);
/*
 * For storage objects that are members of the 'default_lu_gp',
@@ -913,8 +922,8 @@ int core_alua_do_port_transition(
 */
core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
md_buf, new_state, explicit);
-   atomic_dec(&lu_gp->lu_gp_ref_cnt);
-   smp_mb__after_atomic_dec();
+
+   put_alua_lu_gp(lu_gp);
kfree(md_buf);
return 0;
}
@@ -985,8 +994,7 @@ int core_alua_do_port_transition(
l_tg_pt_gp->tg_pt_gp_id, (explicit) ? "explicit" : "implicit",
core_alua_dump_state(new_state));
 
-   atomic_dec(&lu_gp->lu_gp_ref_cnt);
-   smp_mb__after_atomic_dec();
+   put_alua_lu_gp(lu_gp);
kfree(md_buf);
return 0;
 }
@@ -1107,7 +1115,8 @@ core_alua_allocate_lu_gp(const char *name, int def_group)
INIT_LIST_HEAD(&lu_gp->lu_gp_node);
INIT_LIST_HEAD(&lu_gp->lu_gp_mem_list);
spin_lock_init(&lu_gp->lu_gp_lock);
-   atomic_set(&lu_gp->lu_gp_ref_cnt, 0);
+
+   kref_init(&lu_gp->refcount);
 
if (def_group) {
lu_gp->lu_gp_id = alua_lu_gps_counter++;
@@ -1200,13 +1209,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp *lu_gp)
list_del(&lu_gp->lu_gp_node);
alua_lu_gps_count--;
spin_unlock(&lu_gps_lock);
-   /*
-* Allow struct t10_alua_lu_gp * referenced by 
core_alua_get_lu_gp_by_name()
-* in target_core_configfs.c:target_core_store_alua_lu_gp() to be
-* released with core_alua_put_lu_gp_from_name()
-*/
-   while (atomic_read(&lu_gp->lu_gp_ref_cnt))
-   cpu_relax();
+
/*
 * Release reference to struct t10_alua_lu_gp * from all associated
 * struct se_device.
@@ -1241,7 +1244,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp *lu_gp)
}
spin_unlock(&lu_gp->lu_gp_lock);
 
-   kmem_cache_free(t10_alua_lu_gp_cache, lu_gp);
+   put_alua_lu_gp(lu_gp);
 }
 
 void core_alua_free_lu_gp_mem(struct se_device *dev)
@@ -1284,7 +1287,7 @@ struct t10_alua_lu_gp *core_alua_get_lu_gp_by_name(const 
char *name)
continue;
ci = &lu_gp->lu_gp_group.cg_item;
if (!strcmp(config_item_name(ci), name)) {
-   atomic_inc(&lu_gp->lu_gp_ref_cnt);
+   get_alua_lu_gp(lu_gp);
spin_unlock(&lu_gps_lock);
return lu_gp;
}
@@ -1297,7 +1300,7 @@ struct t10_alua_lu_gp *core_alua_get_lu_gp_by_name(const 
char *name)
 void core_alua_put_lu_gp_from_name(struct t10_alua_lu_gp *lu_gp)
 {
spin_lock(&lu_gps_lock);
-   atomic_dec(&lu_gp->lu_gp_ref_cnt);
+   put_alua_lu_gp(lu_gp);
spin_unlock(&lu_gps_lock);
 }
 
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index d5e0da1..5bdf0d5 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -264,7 +264,7 @@ struct t10_alua_lu_gp {
u16 lu_gp_id;
int lu_gp_valid_id;
u32 lu_gp_members;
-   atomic_t lu_gp_ref_cnt;
+   struct kref refcount;
spinlock_t lu_gp_lock;
struct config_group lu_gp_group;
struct list_head lu_gp_node;
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html