* [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
@ 2012-01-02 9:38 민찬호
2012-01-02 9:57 ` Wu Fengguang
0 siblings, 1 reply; 10+ messages in thread
From: 민찬호 @ 2012-01-02 9:38 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: 'Jens Axboe', 'Wu Fengguang',
'Andrew Morton'
from Chanho Min <chanho.min@lge.com>
System may crash in backing-dev.c when removal SCSI device is detached.
bdi task is killed by bdi_unregister()/'khubd', but task's point remains.
Shortly afterward, If 'wb->wakeup_timer' is expired before
del_timer()/bdi_forker_thread,
wakeup_timer_fn() may wake up the dead thread which cause the crash.
'bdi->wb.task' should be NULL as this patch.
Signed-off-by: Chanho Min <chanho.min@lge.com>
---
mm/backing-dev.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 71034f4..4378a5e 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info
*bdi)
if (bdi->wb.task) {
thaw_process(bdi->wb.task);
kthread_stop(bdi->wb.task);
+ bdi->wb.task = NULL;
}
}
--
1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
@ 2012-01-02 9:38 민찬호
0 siblings, 0 replies; 10+ messages in thread
From: 민찬호 @ 2012-01-02 9:38 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: 'Jens Axboe', 'Wu Fengguang',
'Andrew Morton'
from Chanho Min <chanho.min@lge.com>
System may crash in backing-dev.c when removal SCSI device is detached.
bdi task is killed by bdi_unregister()/'khubd', but task's point remains.
Shortly afterward, If 'wb->wakeup_timer' is expired before
del_timer()/bdi_forker_thread,
wakeup_timer_fn() may wake up the dead thread which cause the crash.
'bdi->wb.task' should be NULL as this patch.
Signed-off-by: Chanho Min <chanho.min@lge.com>
---
mm/backing-dev.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 71034f4..4378a5e 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info
*bdi)
if (bdi->wb.task) {
thaw_process(bdi->wb.task);
kthread_stop(bdi->wb.task);
+ bdi->wb.task = NULL;
}
}
--
1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
2012-01-02 9:38 [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached 민찬호
@ 2012-01-02 9:57 ` Wu Fengguang
2012-01-03 3:23 ` Chanho Min
2012-01-03 3:23 ` Chanho Min
0 siblings, 2 replies; 10+ messages in thread
From: Wu Fengguang @ 2012-01-02 9:57 UTC (permalink / raw)
To: ����ȣ
Cc: linux-mm, linux-kernel, 'Jens Axboe',
'Andrew Morton'
On Mon, Jan 02, 2012 at 06:38:21PM +0900, i? 1/2 i? 1/2 i? 1/2 i? 1/2 EGBP wrote:
> from Chanho Min <chanho.min@lge.com>
>
> System may crash in backing-dev.c when removal SCSI device is detached.
> bdi task is killed by bdi_unregister()/'khubd', but task's point remains.
> Shortly afterward, If 'wb->wakeup_timer' is expired before
> del_timer()/bdi_forker_thread,
> wakeup_timer_fn() may wake up the dead thread which cause the crash.
> 'bdi->wb.task' should be NULL as this patch.
Is it some race condition between del_timer() and del_timer_sync()?
bdi_unregister() calls
del_timer_sync
bdi_wb_shutdown
kthread_stop
in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is
no longer called to access the stopped task.
Thanks,
Fengguang
> Signed-off-by: Chanho Min <chanho.min@lge.com>
> ---
> mm/backing-dev.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index 71034f4..4378a5e 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info
> *bdi)
> if (bdi->wb.task) {
> thaw_process(bdi->wb.task);
> kthread_stop(bdi->wb.task);
> + bdi->wb.task = NULL;
> }
> }
>
> --
> 1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
2012-01-02 9:57 ` Wu Fengguang
@ 2012-01-03 3:23 ` Chanho Min
2012-01-03 4:49 ` Wu Fengguang
2012-01-03 3:23 ` Chanho Min
1 sibling, 1 reply; 10+ messages in thread
From: Chanho Min @ 2012-01-03 3:23 UTC (permalink / raw)
To: 'Wu Fengguang'
Cc: linux-mm, linux-kernel, 'Jens Axboe',
'Andrew Morton'
>On Mon, Jan 02, 2012 at 06:38:21PM +0900, ����ȣ wrote:
>> from Chanho Min <chanho.min@lge.com>
>>
>> System may crash in backing-dev.c when removal SCSI device is detached.
>> bdi task is killed by bdi_unregister()/'khubd', but task's point remains.
>> Shortly afterward, If 'wb->wakeup_timer' is expired before
>> del_timer()/bdi_forker_thread,
>> wakeup_timer_fn() may wake up the dead thread which cause the crash.
>> 'bdi->wb.task' should be NULL as this patch.
>
>Is it some race condition between del_timer() and del_timer_sync()?
>
>bdi_unregister() calls
>
> del_timer_sync
> bdi_wb_shutdown
> kthread_stop
>
>in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is
>no longer called to access the stopped task.
>
It is not race condition. This happens when USB is removed during write-access.
bdi_wakeup_thread_delayed is called after kthread_stop, and timer is activated again.
bdi_unregister
kthread_stop
bdi_wakeup_thread_delayed (sys_write mostly calls this)
timer fires
Anyway,Is this safeguard to prevent from waking up killed thread?
Thanks,
Chanho
>Thanks,
>Fengguang
>
>
>> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> ---
>> mm/backing-dev.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>> index 71034f4..4378a5e 100644
>> --- a/mm/backing-dev.c
>> +++ b/mm/backing-dev.c
>> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info
>> *bdi)
>> if (bdi->wb.task) {
>> thaw_process(bdi->wb.task);
>> kthread_stop(bdi->wb.task);
>> + bdi->wb.task = NULL;
>> }
>> }
>>
>> --
>> 1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
2012-01-02 9:57 ` Wu Fengguang
2012-01-03 3:23 ` Chanho Min
@ 2012-01-03 3:23 ` Chanho Min
1 sibling, 0 replies; 10+ messages in thread
From: Chanho Min @ 2012-01-03 3:23 UTC (permalink / raw)
To: 'Wu Fengguang'
Cc: linux-mm, linux-kernel, 'Jens Axboe',
'Andrew Morton'
>On Mon, Jan 02, 2012 at 06:38:21PM +0900, ����ȣ wrote:
>> from Chanho Min <chanho.min@lge.com>
>>
>> System may crash in backing-dev.c when removal SCSI device is detached.
>> bdi task is killed by bdi_unregister()/'khubd', but task's point remains.
>> Shortly afterward, If 'wb->wakeup_timer' is expired before
>> del_timer()/bdi_forker_thread,
>> wakeup_timer_fn() may wake up the dead thread which cause the crash.
>> 'bdi->wb.task' should be NULL as this patch.
>
>Is it some race condition between del_timer() and del_timer_sync()?
>
>bdi_unregister() calls
>
> del_timer_sync
> bdi_wb_shutdown
> kthread_stop
>
>in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is
>no longer called to access the stopped task.
>
It is not race condition. This happens when USB is removed during write-access.
bdi_wakeup_thread_delayed is called after kthread_stop, and timer is activated again.
bdi_unregister
kthread_stop
bdi_wakeup_thread_delayed (sys_write mostly calls this)
timer fires
Anyway,Is this safeguard to prevent from waking up killed thread?
Thanks,
Chanho
>Thanks,
>Fengguang
>
>
>> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> ---
>> mm/backing-dev.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>> index 71034f4..4378a5e 100644
>> --- a/mm/backing-dev.c
>> +++ b/mm/backing-dev.c
>> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info
>> *bdi)
>> if (bdi->wb.task) {
>> thaw_process(bdi->wb.task);
>> kthread_stop(bdi->wb.task);
>> + bdi->wb.task = NULL;
>> }
>> }
>>
>> --
>> 1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
@ 2012-01-03 3:47 'Chanho Min'
0 siblings, 0 replies; 10+ messages in thread
From: 'Chanho Min' @ 2012-01-03 3:47 UTC (permalink / raw)
To: 'Wu Fengguang'
Cc: linux-mm, linux-kernel, 'Jens Axboe',
'Andrew Morton'
>On Mon, Jan 02, 2012 at 06:38:21PM +0900, ȣ wrote:
>> from Chanho Min <chanho.min@lge.com>
>>
>> System may crash in backing-dev.c when removal SCSI device is detached.
>> bdi task is killed by bdi_unregister()/'khubd', but task's point remains.
>> Shortly afterward, If 'wb->wakeup_timer' is expired before
>> del_timer()/bdi_forker_thread,
>> wakeup_timer_fn() may wake up the dead thread which cause the crash.
>> 'bdi->wb.task' should be NULL as this patch.
>
>Is it some race condition between del_timer() and del_timer_sync()?
>
>bdi_unregister() calls
>
> del_timer_sync
> bdi_wb_shutdown
> kthread_stop
>
>in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is no
>longer called to access the stopped task.
>
It is not race condition. This happens when USB is removed during write-access.
bdi_wakeup_thread_delayed is called after kthread_stop, and timer is activated again.
bdi_unregister
kthread_stop
bdi_wakeup_thread_delayed (sys_write mostly calls this)
timer fires
Anyway,Is this safeguard to prevent from waking up killed thread?
Thanks,
Chanho
>Thanks,
>Fengguang
>
>
>> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> ---
>> mm/backing-dev.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/mm/backing-dev.c b/mm/backing-dev.c index
>> 71034f4..4378a5e 100644
>> --- a/mm/backing-dev.c
>> +++ b/mm/backing-dev.c
>> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct
>> backing_dev_info
>> *bdi)
>> if (bdi->wb.task) {
>> thaw_process(bdi->wb.task);
>> kthread_stop(bdi->wb.task);
>> + bdi->wb.task = NULL;
>> }
>> }
>>
>> --
>> 1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
@ 2012-01-03 3:47 'Chanho Min'
0 siblings, 0 replies; 10+ messages in thread
From: 'Chanho Min' @ 2012-01-03 3:47 UTC (permalink / raw)
To: 'Wu Fengguang'
Cc: linux-mm, linux-kernel, 'Jens Axboe',
'Andrew Morton'
>On Mon, Jan 02, 2012 at 06:38:21PM +0900, ȣ wrote:
>> from Chanho Min <chanho.min@lge.com>
>>
>> System may crash in backing-dev.c when removal SCSI device is detached.
>> bdi task is killed by bdi_unregister()/'khubd', but task's point remains.
>> Shortly afterward, If 'wb->wakeup_timer' is expired before
>> del_timer()/bdi_forker_thread,
>> wakeup_timer_fn() may wake up the dead thread which cause the crash.
>> 'bdi->wb.task' should be NULL as this patch.
>
>Is it some race condition between del_timer() and del_timer_sync()?
>
>bdi_unregister() calls
>
> del_timer_sync
> bdi_wb_shutdown
> kthread_stop
>
>in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is no
>longer called to access the stopped task.
>
It is not race condition. This happens when USB is removed during write-access.
bdi_wakeup_thread_delayed is called after kthread_stop, and timer is activated again.
bdi_unregister
kthread_stop
bdi_wakeup_thread_delayed (sys_write mostly calls this)
timer fires
Anyway,Is this safeguard to prevent from waking up killed thread?
Thanks,
Chanho
>Thanks,
>Fengguang
>
>
>> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> ---
>> mm/backing-dev.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/mm/backing-dev.c b/mm/backing-dev.c index
>> 71034f4..4378a5e 100644
>> --- a/mm/backing-dev.c
>> +++ b/mm/backing-dev.c
>> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct
>> backing_dev_info
>> *bdi)
>> if (bdi->wb.task) {
>> thaw_process(bdi->wb.task);
>> kthread_stop(bdi->wb.task);
>> + bdi->wb.task = NULL;
>> }
>> }
>>
>> --
>> 1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
2012-01-03 3:23 ` Chanho Min
@ 2012-01-03 4:49 ` Wu Fengguang
2012-01-03 11:22 ` Chanho Min
2012-01-03 11:22 ` Chanho Min
0 siblings, 2 replies; 10+ messages in thread
From: Wu Fengguang @ 2012-01-03 4:49 UTC (permalink / raw)
To: Chanho Min
Cc: linux-mm, linux-kernel, 'Jens Axboe',
'Andrew Morton', Rabin Vincent, Linus Walleij
On Tue, Jan 03, 2012 at 12:23:44PM +0900, Chanho Min wrote:
> >On Mon, Jan 02, 2012 at 06:38:21PM +0900, i? 1/2 i? 1/2 i? 1/2 i? 1/2 EGBP wrote:
> >> from Chanho Min <chanho.min@lge.com>
> >>
> >> System may crash in backing-dev.c when removal SCSI device is detached.
> >> bdi task is killed by bdi_unregister()/'khubd', but task's point remains.
> >> Shortly afterward, If 'wb->wakeup_timer' is expired before
> >> del_timer()/bdi_forker_thread,
> >> wakeup_timer_fn() may wake up the dead thread which cause the crash.
> >> 'bdi->wb.task' should be NULL as this patch.
> >
> >Is it some race condition between del_timer() and del_timer_sync()?
> >
> >bdi_unregister() calls
> >
> > del_timer_sync
> > bdi_wb_shutdown
> > kthread_stop
> >
> >in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is
> >no longer called to access the stopped task.
> >
>
> It is not race condition. This happens when USB is removed during write-access.
> bdi_wakeup_thread_delayed is called after kthread_stop, and timer is activated again.
>
> bdi_unregister
> kthread_stop
> bdi_wakeup_thread_delayed (sys_write mostly calls this)
> timer fires
Ah OK, the timer could be restarted in the mean while, which breaks
the synchronization rule in del_timer_sync().
I noticed a related fix is merged recently, does your test kernel
contain this commit?
commit 7a401a972df8e184b3d1a3fc958c0a4ddee8d312
Author: Rabin Vincent <rabin.vincent@stericsson.com>
Date: Fri Nov 11 13:29:04 2011 +0100
backing-dev: ensure wakeup_timer is deleted
> Anyway,Is this safeguard to prevent from waking up killed thread?
This patch makes no guarantee wakeup_timer_fn() will see NULL
bdi->wb.task before the task is stopped, so there is still race
conditions. And still, the complete fix would be to prevent
wakeup_timer_fn() from being called at all.
Thanks,
Fengguang
> >> Signed-off-by: Chanho Min <chanho.min@lge.com>
> >> ---
> >> mm/backing-dev.c | 1 +
> >> 1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> >> index 71034f4..4378a5e 100644
> >> --- a/mm/backing-dev.c
> >> +++ b/mm/backing-dev.c
> >> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info
> >> *bdi)
> >> if (bdi->wb.task) {
> >> thaw_process(bdi->wb.task);
> >> kthread_stop(bdi->wb.task);
> >> + bdi->wb.task = NULL;
> >> }
> >> }
> >>
> >> --
> >> 1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
2012-01-03 4:49 ` Wu Fengguang
2012-01-03 11:22 ` Chanho Min
@ 2012-01-03 11:22 ` Chanho Min
1 sibling, 0 replies; 10+ messages in thread
From: Chanho Min @ 2012-01-03 11:22 UTC (permalink / raw)
To: 'Wu Fengguang'
Cc: linux-mm, linux-kernel, 'Jens Axboe',
'Andrew Morton', 'Rabin Vincent',
'Linus Walleij'
>On Tue, Jan 03, 2012 at 12:23:44PM +0900, Chanho Min wrote:
>> >On Mon, Jan 02, 2012 at 06:38:21PM +0900, ����ȣ wrote:
>> >> from Chanho Min <chanho.min@lge.com>
>> >>
>> >> System may crash in backing-dev.c when removal SCSI device is detached.
>> >> bdi task is killed by bdi_unregister()/'khubd', but task's point
>remains.
>> >> Shortly afterward, If 'wb->wakeup_timer' is expired before
>> >> del_timer()/bdi_forker_thread,
>> >> wakeup_timer_fn() may wake up the dead thread which cause the crash.
>> >> 'bdi->wb.task' should be NULL as this patch.
>> >
>> >Is it some race condition between del_timer() and del_timer_sync()?
>> >
>> >bdi_unregister() calls
>> >
>> > del_timer_sync
>> > bdi_wb_shutdown
>> > kthread_stop
>> >
>> >in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is
>> >no longer called to access the stopped task.
>> >
>>
>> It is not race condition. This happens when USB is removed during write-
>access.
>> bdi_wakeup_thread_delayed is called after kthread_stop, and timer is
>activated again.
>>
>> bdi_unregister
>> kthread_stop
>> bdi_wakeup_thread_delayed (sys_write mostly calls this)
>> timer fires
>
>Ah OK, the timer could be restarted in the mean while, which breaks
>the synchronization rule in del_timer_sync().
>
>I noticed a related fix is merged recently, does your test kernel
>contain this commit?
>
No, I will try to reproduce with this patch.
But, bdi_destroy is not called during write-access. Same result is expected.
>commit 7a401a972df8e184b3d1a3fc958c0a4ddee8d312
>Author: Rabin Vincent <rabin.vincent@stericsson.com>
>Date: Fri Nov 11 13:29:04 2011 +0100
>
> backing-dev: ensure wakeup_timer is deleted
>
>> Anyway,Is this safeguard to prevent from waking up killed thread?
>
>This patch makes no guarantee wakeup_timer_fn() will see NULL
>bdi->wb.task before the task is stopped, so there is still race
>conditions. And still, the complete fix would be to prevent
>wakeup_timer_fn() from being called at all.
If wakeup_timer_fn() see NULL bdi->wb.task, wakeup_timer_fn regards task as killed
and wake up forker thread instead of the defined thread.
Is this intended behavior of the bdi?
>
>Thanks,
>Fengguang
>
>> >> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> >> ---
>> >> mm/backing-dev.c | 1 +
>> >> 1 files changed, 1 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>> >> index 71034f4..4378a5e 100644
>> >> --- a/mm/backing-dev.c
>> >> +++ b/mm/backing-dev.c
>> >> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info
>> >> *bdi)
>> >> if (bdi->wb.task) {
>> >> thaw_process(bdi->wb.task);
>> >> kthread_stop(bdi->wb.task);
>> >> + bdi->wb.task = NULL;
>> >> }
>> >> }
>> >>
>> >> --
>> >> 1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached
2012-01-03 4:49 ` Wu Fengguang
@ 2012-01-03 11:22 ` Chanho Min
2012-01-03 11:22 ` Chanho Min
1 sibling, 0 replies; 10+ messages in thread
From: Chanho Min @ 2012-01-03 11:22 UTC (permalink / raw)
To: 'Wu Fengguang'
Cc: linux-mm, linux-kernel, 'Jens Axboe',
'Andrew Morton', 'Rabin Vincent',
'Linus Walleij'
>On Tue, Jan 03, 2012 at 12:23:44PM +0900, Chanho Min wrote:
>> >On Mon, Jan 02, 2012 at 06:38:21PM +0900, ����ȣ wrote:
>> >> from Chanho Min <chanho.min@lge.com>
>> >>
>> >> System may crash in backing-dev.c when removal SCSI device is detached.
>> >> bdi task is killed by bdi_unregister()/'khubd', but task's point
>remains.
>> >> Shortly afterward, If 'wb->wakeup_timer' is expired before
>> >> del_timer()/bdi_forker_thread,
>> >> wakeup_timer_fn() may wake up the dead thread which cause the crash.
>> >> 'bdi->wb.task' should be NULL as this patch.
>> >
>> >Is it some race condition between del_timer() and del_timer_sync()?
>> >
>> >bdi_unregister() calls
>> >
>> > del_timer_sync
>> > bdi_wb_shutdown
>> > kthread_stop
>> >
>> >in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is
>> >no longer called to access the stopped task.
>> >
>>
>> It is not race condition. This happens when USB is removed during write-
>access.
>> bdi_wakeup_thread_delayed is called after kthread_stop, and timer is
>activated again.
>>
>> bdi_unregister
>> kthread_stop
>> bdi_wakeup_thread_delayed (sys_write mostly calls this)
>> timer fires
>
>Ah OK, the timer could be restarted in the mean while, which breaks
>the synchronization rule in del_timer_sync().
>
>I noticed a related fix is merged recently, does your test kernel
>contain this commit?
>
No, I will try to reproduce with this patch.
But, bdi_destroy is not called during write-access. Same result is expected.
>commit 7a401a972df8e184b3d1a3fc958c0a4ddee8d312
>Author: Rabin Vincent <rabin.vincent@stericsson.com>
>Date: Fri Nov 11 13:29:04 2011 +0100
>
> backing-dev: ensure wakeup_timer is deleted
>
>> Anyway,Is this safeguard to prevent from waking up killed thread?
>
>This patch makes no guarantee wakeup_timer_fn() will see NULL
>bdi->wb.task before the task is stopped, so there is still race
>conditions. And still, the complete fix would be to prevent
>wakeup_timer_fn() from being called at all.
If wakeup_timer_fn() see NULL bdi->wb.task, wakeup_timer_fn regards task as killed
and wake up forker thread instead of the defined thread.
Is this intended behavior of the bdi?
>
>Thanks,
>Fengguang
>
>> >> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> >> ---
>> >> mm/backing-dev.c | 1 +
>> >> 1 files changed, 1 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>> >> index 71034f4..4378a5e 100644
>> >> --- a/mm/backing-dev.c
>> >> +++ b/mm/backing-dev.c
>> >> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info
>> >> *bdi)
>> >> if (bdi->wb.task) {
>> >> thaw_process(bdi->wb.task);
>> >> kthread_stop(bdi->wb.task);
>> >> + bdi->wb.task = NULL;
>> >> }
>> >> }
>> >>
>> >> --
>> >> 1.7.0.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-01-03 11:23 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-02 9:38 [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached 민찬호
2012-01-02 9:57 ` Wu Fengguang
2012-01-03 3:23 ` Chanho Min
2012-01-03 4:49 ` Wu Fengguang
2012-01-03 11:22 ` Chanho Min
2012-01-03 11:22 ` Chanho Min
2012-01-03 3:23 ` Chanho Min
-- strict thread matches above, loose matches on Subject: below --
2012-01-02 9:38 민찬호
2012-01-03 3:47 'Chanho Min'
2012-01-03 3:47 'Chanho Min'
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).