* [PATCH] qla2xxx: Fix possible race that could hang kthread_stop()
@ 2011-01-27 20:59 Bandan Das
2011-01-27 21:12 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: Bandan Das @ 2011-01-27 20:59 UTC (permalink / raw)
To: James Bottomley; +Cc: andrew.vasquez, linux-scsi, linux-kernel
There is a small race window in qla2x00_do_dpc() between
checking for kthread_should_stop() and going to sleep after
setting TASK_INTERRUPTIBLE. If qla2x00_free_device() is called
in this window, kthread_stop will wait forever because there
will be no one to wake up the process.
Signed-off-by: Bandan Das <bandan.das@stratus.com>
Signed-off-by: Nate Dailey <nate.dailey@stratus.com>
---
drivers/scsi/qla2xxx/qla_os.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index c194c23..8d14d77 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3286,6 +3286,8 @@ qla2x00_do_dpc(void *data)
DEBUG3(printk("qla2x00: DPC handler sleeping\n"));
set_current_state(TASK_INTERRUPTIBLE);
+ if (kthread_should_stop())
+ break;
schedule();
__set_current_state(TASK_RUNNING);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] qla2xxx: Fix possible race that could hang kthread_stop()
2011-01-27 20:59 [PATCH] qla2xxx: Fix possible race that could hang kthread_stop() Bandan Das
@ 2011-01-27 21:12 ` James Bottomley
2011-01-27 21:32 ` Bandan Das
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2011-01-27 21:12 UTC (permalink / raw)
To: Bandan Das; +Cc: andrew.vasquez, linux-scsi, linux-kernel
On Thu, 2011-01-27 at 15:59 -0500, Bandan Das wrote:
> There is a small race window in qla2x00_do_dpc() between
> checking for kthread_should_stop() and going to sleep after
> setting TASK_INTERRUPTIBLE. If qla2x00_free_device() is called
> in this window, kthread_stop will wait forever because there
> will be no one to wake up the process.
>
>
> Signed-off-by: Bandan Das <bandan.das@stratus.com>
> Signed-off-by: Nate Dailey <nate.dailey@stratus.com>
> ---
> drivers/scsi/qla2xxx/qla_os.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index c194c23..8d14d77 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -3286,6 +3286,8 @@ qla2x00_do_dpc(void *data)
> DEBUG3(printk("qla2x00: DPC handler sleeping\n"));
>
> set_current_state(TASK_INTERRUPTIBLE);
> + if (kthread_should_stop())
> + break;
> schedule();
> __set_current_state(TASK_RUNNING);
That's not really the accepted way to fix these race conditions because
of the double check of kthread_should_stop(); this is
James
---
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index c194c23..15ce69e 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3282,10 +3282,10 @@ qla2x00_do_dpc(void *data)
set_user_nice(current, -20);
+ set_current_state(TASK_INTERRUPTIBLE);
while (!kthread_should_stop()) {
DEBUG3(printk("qla2x00: DPC handler sleeping\n"));
- set_current_state(TASK_INTERRUPTIBLE);
schedule();
__set_current_state(TASK_RUNNING);
@@ -3454,7 +3454,9 @@ qla2x00_do_dpc(void *data)
qla2x00_do_dpc_all_vps(base_vha);
ha->dpc_active = 0;
+ set_current_state(TASK_INTERRUPTIBLE);
} /* End of while(1) */
+ __set_current_state(TASK_RUNNING);
DEBUG(printk("scsi(%ld): DPC handler exiting\n", base_vha->host_no));
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] qla2xxx: Fix possible race that could hang kthread_stop()
2011-01-27 21:12 ` James Bottomley
@ 2011-01-27 21:32 ` Bandan Das
2011-01-27 22:36 ` Madhu Iyengar
0 siblings, 1 reply; 5+ messages in thread
From: Bandan Das @ 2011-01-27 21:32 UTC (permalink / raw)
To: James Bottomley; +Cc: andrew.vasquez, linux-scsi, linux-kernel
> That's not really the accepted way to fix these race conditions because
> of the double check of kthread_should_stop(); this is
> James
>
> ---
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index c194c23..15ce69e 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -3282,10 +3282,10 @@ qla2x00_do_dpc(void *data)
>
> set_user_nice(current, -20);
>
> + set_current_state(TASK_INTERRUPTIBLE);
> while (!kthread_should_stop()) {
> DEBUG3(printk("qla2x00: DPC handler sleeping\n"));
>
> - set_current_state(TASK_INTERRUPTIBLE);
> schedule();
> __set_current_state(TASK_RUNNING);
>
> @@ -3454,7 +3454,9 @@ qla2x00_do_dpc(void *data)
> qla2x00_do_dpc_all_vps(base_vha);
>
> ha->dpc_active = 0;
> + set_current_state(TASK_INTERRUPTIBLE);
> } /* End of while(1) */
> + __set_current_state(TASK_RUNNING);
>
> DEBUG(printk("scsi(%ld): DPC handler exiting\n", base_vha->host_no));
>
Yes, this looks more clean and will take care of the problem I am hitting.
Bandan
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] qla2xxx: Fix possible race that could hang kthread_stop()
2011-01-27 21:32 ` Bandan Das
@ 2011-01-27 22:36 ` Madhu Iyengar
2011-01-27 23:05 ` Bandan Das
0 siblings, 1 reply; 5+ messages in thread
From: Madhu Iyengar @ 2011-01-27 22:36 UTC (permalink / raw)
To: Bandan Das, James Bottomley
Cc: Andrew Vasquez, linux-scsi@vger.kernel.org, Madhu Iyengar,
Giridhar Malavali
We at QLogic are fine with James's fix. Here's my ack:
Acked-by: Madhuranath Iyengar <Madhu.Iyengar@qlogic.com>
Cheers,
Madhu
-----Original Message-----
From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Bandan Das
Sent: Thursday, January 27, 2011 1:33 PM
To: James Bottomley
Cc: Andrew Vasquez; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] qla2xxx: Fix possible race that could hang kthread_stop()
> That's not really the accepted way to fix these race conditions because
> of the double check of kthread_should_stop(); this is
> James
>
> ---
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index c194c23..15ce69e 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -3282,10 +3282,10 @@ qla2x00_do_dpc(void *data)
>
> set_user_nice(current, -20);
>
> + set_current_state(TASK_INTERRUPTIBLE);
> while (!kthread_should_stop()) {
> DEBUG3(printk("qla2x00: DPC handler sleeping\n"));
>
> - set_current_state(TASK_INTERRUPTIBLE);
> schedule();
> __set_current_state(TASK_RUNNING);
>
> @@ -3454,7 +3454,9 @@ qla2x00_do_dpc(void *data)
> qla2x00_do_dpc_all_vps(base_vha);
>
> ha->dpc_active = 0;
> + set_current_state(TASK_INTERRUPTIBLE);
> } /* End of while(1) */
> + __set_current_state(TASK_RUNNING);
>
> DEBUG(printk("scsi(%ld): DPC handler exiting\n", base_vha->host_no));
>
Yes, this looks more clean and will take care of the problem I am hitting.
Bandan
--
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
This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] qla2xxx: Fix possible race that could hang kthread_stop()
2011-01-27 22:36 ` Madhu Iyengar
@ 2011-01-27 23:05 ` Bandan Das
0 siblings, 0 replies; 5+ messages in thread
From: Bandan Das @ 2011-01-27 23:05 UTC (permalink / raw)
To: Madhu Iyengar
Cc: James Bottomley, Andrew Vasquez, linux-scsi@vger.kernel.org,
Giridhar Malavali
On 0, Madhu Iyengar <madhu.iyengar@qlogic.com> wrote:
> We at QLogic are fine with James's fix. Here's my ack:
>
> Acked-by: Madhuranath Iyengar <Madhu.Iyengar@qlogic.com>
>
> Cheers,
> Madhu
James, You are gonna take care of this or do you want me to send
a new patch ? I am fine either ways.
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Bandan Das
> Sent: Thursday, January 27, 2011 1:33 PM
> To: James Bottomley
> Cc: Andrew Vasquez; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] qla2xxx: Fix possible race that could hang kthread_stop()
>
>
> > That's not really the accepted way to fix these race conditions because
> > of the double check of kthread_should_stop(); this is
> > James
> >
> > ---
> > diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> > index c194c23..15ce69e 100644
> > --- a/drivers/scsi/qla2xxx/qla_os.c
> > +++ b/drivers/scsi/qla2xxx/qla_os.c
> > @@ -3282,10 +3282,10 @@ qla2x00_do_dpc(void *data)
> >
> > set_user_nice(current, -20);
> >
> > + set_current_state(TASK_INTERRUPTIBLE);
> > while (!kthread_should_stop()) {
> > DEBUG3(printk("qla2x00: DPC handler sleeping\n"));
> >
> > - set_current_state(TASK_INTERRUPTIBLE);
> > schedule();
> > __set_current_state(TASK_RUNNING);
> >
> > @@ -3454,7 +3454,9 @@ qla2x00_do_dpc(void *data)
> > qla2x00_do_dpc_all_vps(base_vha);
> >
> > ha->dpc_active = 0;
> > + set_current_state(TASK_INTERRUPTIBLE);
> > } /* End of while(1) */
> > + __set_current_state(TASK_RUNNING);
> >
> > DEBUG(printk("scsi(%ld): DPC handler exiting\n", base_vha->host_no));
> >
> Yes, this looks more clean and will take care of the problem I am hitting.
>
>
> Bandan
> --
> 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
>
>
> This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-01-27 23:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-27 20:59 [PATCH] qla2xxx: Fix possible race that could hang kthread_stop() Bandan Das
2011-01-27 21:12 ` James Bottomley
2011-01-27 21:32 ` Bandan Das
2011-01-27 22:36 ` Madhu Iyengar
2011-01-27 23:05 ` Bandan Das
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox