On Fri, 2015-05-22 at 09:57 +0200, Hannes Reinecke wrote:
> On 05/22/2015 09:06 AM, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <n...@linux-iscsi.org>
> >
> > With se_port and t10_alua_tg_pt_gp_member being absored into se_lun,
> > there is no need for an extra lock to protect se_lun->lun_se_dev
> > assignment.
> >
> > Also, convert se_lun->lun_stats to use atomic_long_t within the
> > target_complete_ok_work() completion callback.
> >
> > Reported-by: Christoph Hellwig <h...@lst.de>
> > Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
> > ---
> > drivers/target/target_core_device.c | 1 -
> > drivers/target/target_core_stat.c | 65
> > +++++++++++++++++-----------------
> > drivers/target/target_core_tpg.c | 8 ++---
> > drivers/target/target_core_transport.c | 22 ++++--------
> > include/target/target_core_base.h | 9 +++--
> > 5 files changed, 46 insertions(+), 59 deletions(-)
> >
>
> [ .. ]
>
> > diff --git a/drivers/target/target_core_tpg.c
> > b/drivers/target/target_core_tpg.c
> > index 8764f1c..f60b74e 100644
> > --- a/drivers/target/target_core_tpg.c
> > +++ b/drivers/target/target_core_tpg.c
> > @@ -601,7 +601,6 @@ struct se_lun *core_tpg_alloc_lun(
> > lun->lun_link_magic = SE_LUN_LINK_MAGIC;
> > lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
> > atomic_set(&lun->lun_acl_count, 0);
> > - spin_lock_init(&lun->lun_sep_lock);
> > init_completion(&lun->lun_ref_comp);
> > INIT_LIST_HEAD(&lun->lun_deve_list);
> > INIT_LIST_HEAD(&lun->lun_dev_link);
> > @@ -638,10 +637,7 @@ int core_tpg_add_lun(
> > target_attach_tg_pt_gp(lun, dev->t10_alua.default_tg_pt_gp);
> >
> > mutex_lock(&tpg->tpg_lun_mutex);
> > -
> > - spin_lock(&lun->lun_sep_lock);
> > - lun->lun_se_dev = dev;
> > - spin_unlock(&lun->lun_sep_lock);
> > + rcu_assign_pointer(lun->lun_se_dev, dev);
> >
> > spin_lock(&dev->se_port_lock);
> > dev->export_count++;
> > @@ -683,7 +679,7 @@ void core_tpg_remove_lun(
> > dev->export_count--;
> > spin_unlock(&dev->se_port_lock);
> >
> > - lun->lun_se_dev = NULL;
> > + rcu_assign_pointer(lun->lun_se_dev, NULL);
> > }
> >
> > lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
> Doesn't this need to go under 'dev->se_port_lock' like in
> core_tgp_add_lun() ?
Yes, of course. Adding the following change, and will squash into the
original patch soon.
Thanks for reviewing Hannes!
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index aa08d6b..a3c9121 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -641,9 +641,9 @@ int core_tpg_add_lun(
target_attach_tg_pt_gp(lun, dev->t10_alua.default_tg_pt_gp);
mutex_lock(&tpg->tpg_lun_mutex);
- rcu_assign_pointer(lun->lun_se_dev, dev);
spin_lock(&dev->se_port_lock);
+ rcu_assign_pointer(lun->lun_se_dev, dev);
dev->export_count++;
list_add_tail(&lun->lun_dev_link, &dev->dev_sep_list);
spin_unlock(&dev->se_port_lock);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
