Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
On Mon, Nov 19, 2012 at 10:01 AM, Liu, Chuansheng wrote: >> > Rechecked these codes, the trace event runtime_pm_status is added newly, >> this is different with vanilla >> > Linux. >> >> Not sure I'm following - can you point out which tree are you working with ? > Sorry, it is added internally for debugging purpose. Maybe keep this patch internally too then ? -- 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/
RE: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
> -Original Message- > From: Ohad Ben-Cohen [mailto:o...@wizery.com] > Sent: Monday, November 19, 2012 3:47 PM > To: Liu, Chuansheng > Cc: Chris Ball; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when > calling pm_runtime_set_active() > > On Mon, Nov 19, 2012 at 7:57 AM, Liu, Chuansheng > wrote: > > Rechecked these codes, the trace event runtime_pm_status is added newly, > this is different with vanilla > > Linux. > > Not sure I'm following - can you point out which tree are you working with ? Sorry, it is added internally for debugging purpose. > > > So I still think that calling pm_runtime_set_active is not safe when > > dev_name > is NULL. > > If you agree this point, I can refine the code that moving "init the > > dev_name " > from mmc_add_card > > to mmc_sdio_init_card. > > This sounds more reasonable. I will try a V2 patch soon, thanks. > > Thanks, > Ohad. -- 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/
RE: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
-Original Message- From: Ohad Ben-Cohen [mailto:o...@wizery.com] Sent: Monday, November 19, 2012 3:47 PM To: Liu, Chuansheng Cc: Chris Ball; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active() On Mon, Nov 19, 2012 at 7:57 AM, Liu, Chuansheng chuansheng@intel.com wrote: Rechecked these codes, the trace event runtime_pm_status is added newly, this is different with vanilla Linux. Not sure I'm following - can you point out which tree are you working with ? Sorry, it is added internally for debugging purpose. So I still think that calling pm_runtime_set_active is not safe when dev_name is NULL. If you agree this point, I can refine the code that moving init the dev_name from mmc_add_card to mmc_sdio_init_card. This sounds more reasonable. I will try a V2 patch soon, thanks. Thanks, Ohad. -- 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/
Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
On Mon, Nov 19, 2012 at 10:01 AM, Liu, Chuansheng chuansheng@intel.com wrote: Rechecked these codes, the trace event runtime_pm_status is added newly, this is different with vanilla Linux. Not sure I'm following - can you point out which tree are you working with ? Sorry, it is added internally for debugging purpose. Maybe keep this patch internally too then ? -- 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/
Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
On Mon, Nov 19, 2012 at 7:57 AM, Liu, Chuansheng wrote: > Rechecked these codes, the trace event runtime_pm_status is added newly, this > is different with vanilla > Linux. Not sure I'm following - can you point out which tree are you working with ? > So I still think that calling pm_runtime_set_active is not safe when dev_name > is NULL. > If you agree this point, I can refine the code that moving "init the dev_name > " from mmc_add_card > to mmc_sdio_init_card. This sounds more reasonable. Thanks, Ohad. -- 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/
RE: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
> -Original Message- > From: Ohad Ben-Cohen [mailto:o...@wizery.com] > Sent: Saturday, November 17, 2012 3:33 AM > To: Liu, Chuansheng > Cc: Chris Ball; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when > calling pm_runtime_set_active() > > Hi Liu, > > On Fri, Nov 16, 2012 at 2:54 PM, Chuansheng Liu > wrote: > > So when calling pm_runtime_set_active(), it will hit the strlen(devname==0) > > which trigger the panic. > > Can you please point to the exact line of code that triggers this panic ? The call trace is as below: mmc_rescan -> mmc_rescan_try_freq -> mmc_attach_sdio -> pm_runtime_set_active -> __pm_runtime_set_status -> __update_runtime_status -> trace_runtime_pm_status This function is corresponding to the below code in trace/events/power.h: TRACE_EVENT(runtime_pm_status, TP_PROTO(struct device *dev, int status), TP_ARGS(dev, status), TP_STRUCT__entry( __string(devname, dev_name(dev)) Rechecked these codes, the trace event runtime_pm_status is added newly, this is different with vanilla Linux. But it is just a new trace event. So I still think that calling pm_runtime_set_active is not safe when dev_name is NULL. If you agree this point, I can refine the code that moving "init the dev_name " from mmc_add_card to mmc_sdio_init_card. Thanks. > > > Here before calling pm_runtime_set_active(), set the dev name, although > > it is duplicated with mmc_add_card(), but it do not break the original > > design(commit 81968561b). > > Normally we'd like to avoid such a solution, so let's first make sure > we understand the problem. > > Have you tried thinking how come this shows up only now - has any of > the relevant code been changed lately ? Are you using a vanilla Linux > tree ? > > Thanks, > Ohad. -- 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/
RE: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
-Original Message- From: Ohad Ben-Cohen [mailto:o...@wizery.com] Sent: Saturday, November 17, 2012 3:33 AM To: Liu, Chuansheng Cc: Chris Ball; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active() Hi Liu, On Fri, Nov 16, 2012 at 2:54 PM, Chuansheng Liu chuansheng@intel.com wrote: So when calling pm_runtime_set_active(), it will hit the strlen(devname==0) which trigger the panic. Can you please point to the exact line of code that triggers this panic ? The call trace is as below: mmc_rescan - mmc_rescan_try_freq - mmc_attach_sdio - pm_runtime_set_active - __pm_runtime_set_status - __update_runtime_status - trace_runtime_pm_status This function is corresponding to the below code in trace/events/power.h: TRACE_EVENT(runtime_pm_status, TP_PROTO(struct device *dev, int status), TP_ARGS(dev, status), TP_STRUCT__entry( __string(devname, dev_name(dev)) Rechecked these codes, the trace event runtime_pm_status is added newly, this is different with vanilla Linux. But it is just a new trace event. So I still think that calling pm_runtime_set_active is not safe when dev_name is NULL. If you agree this point, I can refine the code that moving init the dev_name from mmc_add_card to mmc_sdio_init_card. Thanks. Here before calling pm_runtime_set_active(), set the dev name, although it is duplicated with mmc_add_card(), but it do not break the original design(commit 81968561b). Normally we'd like to avoid such a solution, so let's first make sure we understand the problem. Have you tried thinking how come this shows up only now - has any of the relevant code been changed lately ? Are you using a vanilla Linux tree ? Thanks, Ohad. -- 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/
Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
On Mon, Nov 19, 2012 at 7:57 AM, Liu, Chuansheng chuansheng@intel.com wrote: Rechecked these codes, the trace event runtime_pm_status is added newly, this is different with vanilla Linux. Not sure I'm following - can you point out which tree are you working with ? So I still think that calling pm_runtime_set_active is not safe when dev_name is NULL. If you agree this point, I can refine the code that moving init the dev_name from mmc_add_card to mmc_sdio_init_card. This sounds more reasonable. Thanks, Ohad. -- 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/
Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
Hi Liu, On Fri, Nov 16, 2012 at 2:54 PM, Chuansheng Liu wrote: > So when calling pm_runtime_set_active(), it will hit the strlen(devname==0) > which trigger the panic. Can you please point to the exact line of code that triggers this panic ? > Here before calling pm_runtime_set_active(), set the dev name, although > it is duplicated with mmc_add_card(), but it do not break the original > design(commit 81968561b). Normally we'd like to avoid such a solution, so let's first make sure we understand the problem. Have you tried thinking how come this shows up only now - has any of the relevant code been changed lately ? Are you using a vanilla Linux tree ? Thanks, Ohad. -- 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/
Re: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
Hi Liu, On Fri, Nov 16, 2012 at 2:54 PM, Chuansheng Liu chuansheng@intel.com wrote: So when calling pm_runtime_set_active(), it will hit the strlen(devname==0) which trigger the panic. Can you please point to the exact line of code that triggers this panic ? Here before calling pm_runtime_set_active(), set the dev name, although it is duplicated with mmc_add_card(), but it do not break the original design(commit 81968561b). Normally we'd like to avoid such a solution, so let's first make sure we understand the problem. Have you tried thinking how come this shows up only now - has any of the relevant code been changed lately ? Are you using a vanilla Linux tree ? Thanks, Ohad. -- 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/
[PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
Subject: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling
pm_runtime_set_active()
Meet one panic as the below:
<1>[ 15.067350] BUG: unable to handle kernel NULL pointer dereference at
(null)
<1>[ 15.074455] IP: [] strlen+0x12/0x20
<4>[ 15.078803] *pde =
<0>[ 15.081674] Oops: [#1] PREEMPT SMP
<4>[ 15.101676] Pid: 5, comm: kworker/u:0 Tainted: G C
3.0.34-140729-g7f9d5c5 #1 Intel Corporation Medfield/BKB2
<4>[ 15.112282] EIP: 0060:[] EFLAGS: 00010046 CPU: 0
<4>[ 15.117760] EIP is at strlen+0x12/0x20
<4>[ 15.121496] EAX: EBX: f344cc04 ECX: EDX: f344cc04
<4>[ 15.127754] ESI: c12bcee0 EDI: EBP: f586fe74 ESP: f586fe70
<4>[ 15.134013] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068
<0>[ 15.139406] Process kworker/u:0 (pid: 5, ti=f586e000 task=f585b440
task.ti=f586e000)
<0>[ 15.147140] Stack:
<4>[ 15.149141] f344cc04 f586feb0 c12bcf12 f586fe9c
0007 0082
<4>[ 15.156877] 0092 0002 c1b01ee4 f586feb8 c1635f31 f3b42330
c12bcee0 f344cc04
<4>[ 15.164616] f586fed0 c152f815 f3b42330 f3b42328 f344cc04
f589b804
<0>[ 15.172351] Call Trace:
<4>[ 15.174810] [] ftrace_raw_event_runtime_pm_status+0x32/0x140
<4>[ 15.181411] [] ? sdio_enable_wide.part.8+0x61/0x80
<4>[ 15.187145] [] ? perf_trace_runtime_pm_usage+0x1a0/0x1a0
<4>[ 15.193407] [] __update_runtime_status+0x65/0x90
<4>[ 15.198968] [] __pm_runtime_set_status+0xe0/0x1b0
<4>[ 15.204621] [] mmc_attach_sdio+0x2f6/0x410
<4>[ 15.209666] [] mmc_rescan+0x240/0x2b0
<4>[ 15.214270] [] process_one_work+0xfe/0x3f0
<4>[ 15.219311] [] ? wake_up_process+0x14/0x20
<4>[ 15.224357] [] ? mmc_detect_card_removed+0x80/0x80
<4>[ 15.230091] [] worker_thread+0x121/0x2f0
<4>[ 15.234958] [] ? rescuer_thread+0x1e0/0x1e0
<4>[ 15.240091] [] kthread+0x6d/0x80
<4>[ 15.244264] [] ? __init_kthread_worker+0x30/0x30
<4>[ 15.245485] [] kernel_thread_helper+0x6/0x10
The reason is pm_runtime_set_active() is called before the device name
is set, and the dev name setting is done at mmc_add_card() laterly.
So when calling pm_runtime_set_active(), it will hit the strlen(devname==0)
which trigger the panic.
Here before calling pm_runtime_set_active(), set the dev name, although
it is duplicated with mmc_add_card(), but it do not break the original
design(commit 81968561b).
Signed-off-by: liu chuansheng
---
drivers/mmc/core/sdio.c |9 +
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 2273ce6..73746af 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -1104,6 +1104,15 @@ int mmc_attach_sdio(struct mmc_host *host)
*/
if (host->caps & MMC_CAP_POWER_OFF_CARD) {
/*
+* pm_runtime_set_active will use strlen(dev_name),
+* we must set it in advance to avoid crash,
+* although it is the duplication in mmc_add_card
+* laterly.
+*/
+ dev_set_name(>dev, "%s:%04x", mmc_hostname(card->host),
+ card->rca);
+
+ /*
* Let runtime PM core know our card is active
*/
err = pm_runtime_set_active(>dev);
--
1.7.0.4
--
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/
[PATCH] mmc,sdio: Fix the panic due to devname NULL when calling pm_runtime_set_active()
Subject: [PATCH] mmc,sdio: Fix the panic due to devname NULL when calling
pm_runtime_set_active()
Meet one panic as the below:
1[ 15.067350] BUG: unable to handle kernel NULL pointer dereference at
(null)
1[ 15.074455] IP: [c1496a42] strlen+0x12/0x20
4[ 15.078803] *pde =
0[ 15.081674] Oops: [#1] PREEMPT SMP
4[ 15.101676] Pid: 5, comm: kworker/u:0 Tainted: G C
3.0.34-140729-g7f9d5c5 #1 Intel Corporation Medfield/BKB2
4[ 15.112282] EIP: 0060:[c1496a42] EFLAGS: 00010046 CPU: 0
4[ 15.117760] EIP is at strlen+0x12/0x20
4[ 15.121496] EAX: EBX: f344cc04 ECX: EDX: f344cc04
4[ 15.127754] ESI: c12bcee0 EDI: EBP: f586fe74 ESP: f586fe70
4[ 15.134013] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068
0[ 15.139406] Process kworker/u:0 (pid: 5, ti=f586e000 task=f585b440
task.ti=f586e000)
0[ 15.147140] Stack:
4[ 15.149141] f344cc04 f586feb0 c12bcf12 f586fe9c
0007 0082
4[ 15.156877] 0092 0002 c1b01ee4 f586feb8 c1635f31 f3b42330
c12bcee0 f344cc04
4[ 15.164616] f586fed0 c152f815 f3b42330 f3b42328 f344cc04
f589b804
0[ 15.172351] Call Trace:
4[ 15.174810] [c12bcf12] ftrace_raw_event_runtime_pm_status+0x32/0x140
4[ 15.181411] [c1635f31] ? sdio_enable_wide.part.8+0x61/0x80
4[ 15.187145] [c12bcee0] ? perf_trace_runtime_pm_usage+0x1a0/0x1a0
4[ 15.193407] [c152f815] __update_runtime_status+0x65/0x90
4[ 15.198968] [c1531170] __pm_runtime_set_status+0xe0/0x1b0
4[ 15.204621] [c1637366] mmc_attach_sdio+0x2f6/0x410
4[ 15.209666] [c162f520] mmc_rescan+0x240/0x2b0
4[ 15.214270] [c12643ce] process_one_work+0xfe/0x3f0
4[ 15.219311] [c1242754] ? wake_up_process+0x14/0x20
4[ 15.224357] [c162f2e0] ? mmc_detect_card_removed+0x80/0x80
4[ 15.230091] [c12649c1] worker_thread+0x121/0x2f0
4[ 15.234958] [c12648a0] ? rescuer_thread+0x1e0/0x1e0
4[ 15.240091] [c12684cd] kthread+0x6d/0x80
4[ 15.244264] [c1268460] ? __init_kthread_worker+0x30/0x30
4[ 15.245485] [c186dc3a] kernel_thread_helper+0x6/0x10
The reason is pm_runtime_set_active() is called before the device name
is set, and the dev name setting is done at mmc_add_card() laterly.
So when calling pm_runtime_set_active(), it will hit the strlen(devname==0)
which trigger the panic.
Here before calling pm_runtime_set_active(), set the dev name, although
it is duplicated with mmc_add_card(), but it do not break the original
design(commit 81968561b).
Signed-off-by: liu chuansheng chuansheng@intel.com
---
drivers/mmc/core/sdio.c |9 +
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 2273ce6..73746af 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -1104,6 +1104,15 @@ int mmc_attach_sdio(struct mmc_host *host)
*/
if (host-caps MMC_CAP_POWER_OFF_CARD) {
/*
+* pm_runtime_set_active will use strlen(dev_name),
+* we must set it in advance to avoid crash,
+* although it is the duplication in mmc_add_card
+* laterly.
+*/
+ dev_set_name(card-dev, %s:%04x, mmc_hostname(card-host),
+ card-rca);
+
+ /*
* Let runtime PM core know our card is active
*/
err = pm_runtime_set_active(card-dev);
--
1.7.0.4
--
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/
