| Message ID | 20220215182650.19839-3-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix a deadlock in the ib_srp driver | expand |
On Tue, Feb 15, 2022 at 10:26:49AM -0800, Bart Van Assche wrote: > This patch makes the next patch in this series easier to read. It is not readability change, but move to lock that can sleep (mutex), which is always good thing. Reviewed-by: Leon Romanovsky <leonro@nvidia.com> > > Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/infiniband/ulp/srp/ib_srp.c | 18 +++++++++--------- > drivers/infiniband/ulp/srp/ib_srp.h | 4 ++-- > 2 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c > index e174e853f8a4..2db7429b42e1 100644 > --- a/drivers/infiniband/ulp/srp/ib_srp.c > +++ b/drivers/infiniband/ulp/srp/ib_srp.c > @@ -1062,9 +1062,9 @@ static void srp_remove_target(struct srp_target_port *target) > kfree(target->ch); > target->ch = NULL; > > - spin_lock(&target->srp_host->target_lock); > + mutex_lock(&target->srp_host->target_mutex); > list_del(&target->list); > - spin_unlock(&target->srp_host->target_lock); > + mutex_unlock(&target->srp_host->target_mutex); > > scsi_host_put(target->scsi_host); > } > @@ -3146,9 +3146,9 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target) > rport->lld_data = target; > target->rport = rport; > > - spin_lock(&host->target_lock); > + mutex_lock(&host->target_mutex); > list_add_tail(&target->list, &host->target_list); > - spin_unlock(&host->target_lock); > + mutex_unlock(&host->target_mutex); > > scsi_scan_target(&target->scsi_host->shost_gendev, > 0, target->scsi_id, SCAN_WILD_CARD, SCSI_SCAN_INITIAL); > @@ -3203,7 +3203,7 @@ static bool srp_conn_unique(struct srp_host *host, > > ret = true; > > - spin_lock(&host->target_lock); > + mutex_lock(&host->target_mutex); > list_for_each_entry(t, &host->target_list, list) { > if (t != target && > target->id_ext == t->id_ext && > @@ -3213,7 +3213,7 @@ static bool srp_conn_unique(struct srp_host *host, > break; > } > } > - spin_unlock(&host->target_lock); > + mutex_unlock(&host->target_mutex); > > out: > return ret; > @@ -3898,7 +3898,7 @@ static struct srp_host *srp_add_port(struct srp_device *device, u8 port) > return NULL; > > INIT_LIST_HEAD(&host->target_list); > - spin_lock_init(&host->target_lock); > + mutex_init(&host->target_mutex); > init_completion(&host->released); > mutex_init(&host->add_target_mutex); > host->srp_dev = device; > @@ -4041,10 +4041,10 @@ static void srp_remove_one(struct ib_device *device, void *client_data) > /* > * Remove all target ports. > */ > - spin_lock(&host->target_lock); > + mutex_lock(&host->target_mutex); > list_for_each_entry(target, &host->target_list, list) > srp_queue_remove_work(target); > - spin_unlock(&host->target_lock); > + mutex_unlock(&host->target_mutex); > > /* > * Wait for tl_err and target port removal tasks. > diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h > index 55a575e2cace..2af968277994 100644 > --- a/drivers/infiniband/ulp/srp/ib_srp.h > +++ b/drivers/infiniband/ulp/srp/ib_srp.h > @@ -116,14 +116,14 @@ struct srp_device { > * One port of an RDMA adapter in the initiator system. > * > * @target_list: List of connected target ports (struct srp_target_port). > - * @target_lock: Protects @target_list. > + * @target_mutex: Protects @target_list. > */ > struct srp_host { > struct srp_device *srp_dev; > u8 port; > struct device dev; > struct list_head target_list; > - spinlock_t target_lock; > + struct mutex target_mutex; > struct completion released; > struct list_head list; > struct mutex add_target_mutex;
On 2/15/22 10:44, Leon Romanovsky wrote: > On Tue, Feb 15, 2022 at 10:26:49AM -0800, Bart Van Assche wrote: >> This patch makes the next patch in this series easier to read. > > It is not readability change, but move to lock that can sleep (mutex), > which is always good thing. > > Reviewed-by: Leon Romanovsky <leonro@nvidia.com> Thanks for the review, but please note that I wrote that this patch improves readability of the next patch in this series. Bart.
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index e174e853f8a4..2db7429b42e1 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1062,9 +1062,9 @@ static void srp_remove_target(struct srp_target_port *target) kfree(target->ch); target->ch = NULL; - spin_lock(&target->srp_host->target_lock); + mutex_lock(&target->srp_host->target_mutex); list_del(&target->list); - spin_unlock(&target->srp_host->target_lock); + mutex_unlock(&target->srp_host->target_mutex); scsi_host_put(target->scsi_host); } @@ -3146,9 +3146,9 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target) rport->lld_data = target; target->rport = rport; - spin_lock(&host->target_lock); + mutex_lock(&host->target_mutex); list_add_tail(&target->list, &host->target_list); - spin_unlock(&host->target_lock); + mutex_unlock(&host->target_mutex); scsi_scan_target(&target->scsi_host->shost_gendev, 0, target->scsi_id, SCAN_WILD_CARD, SCSI_SCAN_INITIAL); @@ -3203,7 +3203,7 @@ static bool srp_conn_unique(struct srp_host *host, ret = true; - spin_lock(&host->target_lock); + mutex_lock(&host->target_mutex); list_for_each_entry(t, &host->target_list, list) { if (t != target && target->id_ext == t->id_ext && @@ -3213,7 +3213,7 @@ static bool srp_conn_unique(struct srp_host *host, break; } } - spin_unlock(&host->target_lock); + mutex_unlock(&host->target_mutex); out: return ret; @@ -3898,7 +3898,7 @@ static struct srp_host *srp_add_port(struct srp_device *device, u8 port) return NULL; INIT_LIST_HEAD(&host->target_list); - spin_lock_init(&host->target_lock); + mutex_init(&host->target_mutex); init_completion(&host->released); mutex_init(&host->add_target_mutex); host->srp_dev = device; @@ -4041,10 +4041,10 @@ static void srp_remove_one(struct ib_device *device, void *client_data) /* * Remove all target ports. */ - spin_lock(&host->target_lock); + mutex_lock(&host->target_mutex); list_for_each_entry(target, &host->target_list, list) srp_queue_remove_work(target); - spin_unlock(&host->target_lock); + mutex_unlock(&host->target_mutex); /* * Wait for tl_err and target port removal tasks. diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h index 55a575e2cace..2af968277994 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.h +++ b/drivers/infiniband/ulp/srp/ib_srp.h @@ -116,14 +116,14 @@ struct srp_device { * One port of an RDMA adapter in the initiator system. * * @target_list: List of connected target ports (struct srp_target_port). - * @target_lock: Protects @target_list. + * @target_mutex: Protects @target_list. */ struct srp_host { struct srp_device *srp_dev; u8 port; struct device dev; struct list_head target_list; - spinlock_t target_lock; + struct mutex target_mutex; struct completion released; struct list_head list; struct mutex add_target_mutex;
This patch makes the next patch in this series easier to read. Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/infiniband/ulp/srp/ib_srp.c | 18 +++++++++--------- drivers/infiniband/ulp/srp/ib_srp.h | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-)