| Message ID | 1476340729-8569-2-git-send-email-jitendra.bhivare@broadcom.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 10/13/2016 08:38 AM, Jitendra Bhivare wrote: > The code at free_task label in __iscsi_conn_send_pdu can get executed > from blk_timeout_work which takes queue_lock using spin_lock_irq. > back_lock taken with spin_unlock_bh will cause WARN_ON_ONCE. > The code gets executed either with bottom half or IRQ disabled hence > using spin_lock/spin_unlock for back_lock is safe. > > Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com> > --- > drivers/scsi/libiscsi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c > index c051694..f9b6fba 100644 > --- a/drivers/scsi/libiscsi.c > +++ b/drivers/scsi/libiscsi.c > @@ -791,9 +791,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr, > > free_task: > /* regular RX path uses back_lock */ > - spin_lock_bh(&session->back_lock); > + spin_lock(&session->back_lock); > __iscsi_put_task(task); > - spin_unlock_bh(&session->back_lock); > + spin_unlock(&session->back_lock); > return NULL; > } > > Reviewed-by: Hannes Reinecke <hare@suse.com> Cheers, Hannes
On Thu, Oct 13, 2016 at 12:08:48PM +0530, Jitendra Bhivare wrote: > The code at free_task label in __iscsi_conn_send_pdu can get executed > from blk_timeout_work which takes queue_lock using spin_lock_irq. > back_lock taken with spin_unlock_bh will cause WARN_ON_ONCE. > The code gets executed either with bottom half or IRQ disabled hence > using spin_lock/spin_unlock for back_lock is safe. > > Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com> > --- > drivers/scsi/libiscsi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c > index c051694..f9b6fba 100644 > --- a/drivers/scsi/libiscsi.c > +++ b/drivers/scsi/libiscsi.c > @@ -791,9 +791,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr, > > free_task: > /* regular RX path uses back_lock */ > - spin_lock_bh(&session->back_lock); > + spin_lock(&session->back_lock); > __iscsi_put_task(task); > - spin_unlock_bh(&session->back_lock); > + spin_unlock(&session->back_lock); > return NULL; > } > Thanks for going back and verifying that irqsave wasn't needed here, I was concerned about adding more interrupt disabled sections outside of the error handling paths. Reviewed-by: Chris Leech <cleech@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/12/2016 11:38 PM, Jitendra Bhivare wrote: > The code at free_task label in __iscsi_conn_send_pdu can get executed > from blk_timeout_work which takes queue_lock using spin_lock_irq. > back_lock taken with spin_unlock_bh will cause WARN_ON_ONCE. > The code gets executed either with bottom half or IRQ disabled hence > using spin_lock/spin_unlock for back_lock is safe. > > Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com> > --- > drivers/scsi/libiscsi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c > index c051694..f9b6fba 100644 > --- a/drivers/scsi/libiscsi.c > +++ b/drivers/scsi/libiscsi.c > @@ -791,9 +791,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr, > > free_task: > /* regular RX path uses back_lock */ > - spin_lock_bh(&session->back_lock); > + spin_lock(&session->back_lock); > __iscsi_put_task(task); > - spin_unlock_bh(&session->back_lock); > + spin_unlock(&session->back_lock); > return NULL; > } > > Reviewed-by: Lee Duncan <lduncan@suse.com> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index c051694..f9b6fba 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -791,9 +791,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr, free_task: /* regular RX path uses back_lock */ - spin_lock_bh(&session->back_lock); + spin_lock(&session->back_lock); __iscsi_put_task(task); - spin_unlock_bh(&session->back_lock); + spin_unlock(&session->back_lock); return NULL; }
The code at free_task label in __iscsi_conn_send_pdu can get executed from blk_timeout_work which takes queue_lock using spin_lock_irq. back_lock taken with spin_unlock_bh will cause WARN_ON_ONCE. The code gets executed either with bottom half or IRQ disabled hence using spin_lock/spin_unlock for back_lock is safe. Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com> --- drivers/scsi/libiscsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)