* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 13:34 Evgeniy Didin
2018-02-22 15:28 ` Shawn Lin
2018-02-23 3:24 ` Jisheng Zhang
0 siblings, 2 replies; 6+ messages in thread
From: Evgeniy Didin @ 2018-02-22 13:34 UTC (permalink / raw)
To: linux-snps-arc
In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
changes which cause multiply overflow for 32-bit systems.
The broken timeout calculations caused false interrupt latency warnings
and stacktrace splat (such as below) when accessing the SD Card.
| Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
| - Info: Finished target initialization.
| mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
| 0x900, card status 0x0
| mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
| 396825HZ div = 63)
| mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
| 25000000HZ div = 1)
| ------------[ cut here ]------------
| softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
| exited with 00000100?
| WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
| Modules linked in:
| CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
|
| Stack Trace:
| arc_unwind_core.constprop.1+0xd0/0xf4
| dump_stack+0x68/0x80
| warn_slowpath_null+0x4e/0xec
| sg_miter_next+0x28/0x20c
| dw_mci_read_data_pio+0x44/0x190
| dw_mmc f000a000.mmc: Unexpected interrupt latency
| dw_mci_interrupt+0x3ee/0x530
| __handle_irq_event_percpu+0x56/0x150
| handle_irq_event+0x34/0x78
| handle_level_irq+0x8e/0x120
| generic_handle_irq+0x1c/0x2c
| idu_cascade_isr+0x30/0x6c
| __handle_domain_irq+0x72/0xc8
| ret_from_exception+0x0/0x8
|---[ end trace 2a58c9af6c25fe51 ]---
Lets cast this multiply to u64 type which prevents overflow.
Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
CC: Alexey Brodkin <abrodkin at synopsys.com>
CC: Eugeniy Paltsev <paltsev at synopsys.com>
CC: Douglas Anderson <dianders at chromium.org>
CC: Ulf Hansson <ulf.hansson at linaro.org>
CC: linux-kernel at vger.kernel.org
CC: linux-snps-arc at lists.infradead.org
Cc: <stable at vger.kernel.org> # 9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
---
Changes since v1:
-uint64_t switched to u64
-changed commit message
drivers/mmc/host/dw_mmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 0aa39975f33b..1a0b9751c67c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1944,7 +1944,7 @@ static void dw_mci_set_drto(struct dw_mci *host)
drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
if (drto_div == 0)
drto_div = 1;
- drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
+ drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
host->bus_hz);
/* add a bit spare time */
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
2018-02-22 13:34 [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems Evgeniy Didin
@ 2018-02-22 15:28 ` Shawn Lin
2018-02-22 16:03 ` Alexey Brodkin
2018-02-22 17:38 ` Vineet Gupta
2018-02-23 3:24 ` Jisheng Zhang
1 sibling, 2 replies; 6+ messages in thread
From: Shawn Lin @ 2018-02-22 15:28 UTC (permalink / raw)
To: linux-snps-arc
[+ Jaehoon]
On 2018/2/22 21:34, Evgeniy Didin wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
> changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations caused false interrupt latency warnings
> and stacktrace splat (such as below) when accessing the SD Card.
>
> | Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
> | - Info: Finished target initialization.
> | mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
> | 0x900, card status 0x0
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
> | 396825HZ div = 63)
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
> | 25000000HZ div = 1)
> | ------------[ cut here ]------------
> | softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
> | exited with 00000100?
> | WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
> | Modules linked in:
> | CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
> |
> | Stack Trace:
> | arc_unwind_core.constprop.1+0xd0/0xf4
> | dump_stack+0x68/0x80
> | warn_slowpath_null+0x4e/0xec
> | sg_miter_next+0x28/0x20c
> | dw_mci_read_data_pio+0x44/0x190
> | dw_mmc f000a000.mmc: Unexpected interrupt latency
I think we tested SD cards but the main reason we missed
this is that we don't use pio mode since dw_mmc decides
the transfer mode via HCON register but we don't have one
platform at hand then to do that. Given the data-transfer-over
interrupt should come after the data hit the RAM, so pio mode
could probably consume more time than IDMAC.
> | dw_mci_interrupt+0x3ee/0x530
> | __handle_irq_event_percpu+0x56/0x150
> | handle_irq_event+0x34/0x78
> | handle_level_irq+0x8e/0x120
> | generic_handle_irq+0x1c/0x2c
> | idu_cascade_isr+0x30/0x6c
> | __handle_domain_irq+0x72/0xc8
> | ret_from_exception+0x0/0x8
> |---[ end trace 2a58c9af6c25fe51 ]---
>
> Lets cast this multiply to u64 type which prevents overflow.
>
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
>
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>
> CC: Alexey Brodkin <abrodkin at synopsys.com>
> CC: Eugeniy Paltsev <paltsev at synopsys.com>
> CC: Douglas Anderson <dianders at chromium.org>
> CC: Ulf Hansson <ulf.hansson at linaro.org>
> CC: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org> # 9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
The fix looks good to me, but the tags should be improved a bit:
Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
Cc: <stable at vger.kernel.org>
Otherwise,
Reviewed-by: Shawn Lin <shawn.lin at rock-chips.com>
BTW, would you mind cooking another patch for fixing cto_ms case?
> ---
> Changes since v1:
> -uint64_t switched to u64
> -changed commit message
> drivers/mmc/host/dw_mmc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..1a0b9751c67c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1944,7 +1944,7 @@ static void dw_mci_set_drto(struct dw_mci *host)
> drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (drto_div == 0)
> drto_div = 1;
> - drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
> + drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
> host->bus_hz);
>
> /* add a bit spare time */
>
--
Best Regards
Shawn Lin
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
2018-02-22 15:28 ` Shawn Lin
@ 2018-02-22 16:03 ` Alexey Brodkin
2018-02-22 16:36 ` Shawn Lin
2018-02-22 17:38 ` Vineet Gupta
1 sibling, 1 reply; 6+ messages in thread
From: Alexey Brodkin @ 2018-02-22 16:03 UTC (permalink / raw)
To: linux-snps-arc
Hi Shawn,
On Thu, 2018-02-22@23:28 +0800, Shawn Lin wrote:
[snip]
> > > Stack Trace:
> > > arc_unwind_core.constprop.1+0xd0/0xf4
> > > dump_stack+0x68/0x80
> > > warn_slowpath_null+0x4e/0xec
> > > sg_miter_next+0x28/0x20c
> > > dw_mci_read_data_pio+0x44/0x190
> > > dw_mmc f000a000.mmc: Unexpected interrupt latency
>
> I think we tested SD cards but the main reason we missed
> this is that we don't use pio mode since dw_mmc decides
> the transfer mode via HCON register but we don't have one
> platform at hand then to do that. Given the data-transfer-over
> interrupt should come after the data hit the RAM, so pio mode
> could probably consume more time than IDMAC.
That's really interesting.
I was under impression that we use internal DMA controller (AKA IDMAC)
on that platform (HSDK).
This is what we typically see in bootlog (this extract is taken from
4.15-r9):
--------------------------------->8--------------------------------
dw_mmc f000a000.mmc: 'num-slots' was deprecated.
dw_mmc f000a000.mmc: IDMAC supports 32-bit address mode.
dw_mmc f000a000.mmc: Using internal DMA controller.
dw_mmc f000a000.mmc: Version ID is 290a
dw_mmc f000a000.mmc: DW MMC controller at irq 12,32 bit host data width,16 deep fifo
--------------------------------->8--------------------------------
I'm not really sure how PIO mode (which stands for non-DMA mode) got used
given we have IDMAC in the hardware.
@ Evgeniy, could you please check why IDMAC is not used?
-Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
2018-02-22 16:03 ` Alexey Brodkin
@ 2018-02-22 16:36 ` Shawn Lin
0 siblings, 0 replies; 6+ messages in thread
From: Shawn Lin @ 2018-02-22 16:36 UTC (permalink / raw)
To: linux-snps-arc
On 2018/2/23 0:03, Alexey Brodkin wrote:
> Hi Shawn,
>
> On Thu, 2018-02-22@23:28 +0800, Shawn Lin wrote:
>
> [snip]
>
>>>> Stack Trace:
>>>> arc_unwind_core.constprop.1+0xd0/0xf4
>>>> dump_stack+0x68/0x80
>>>> warn_slowpath_null+0x4e/0xec
>>>> sg_miter_next+0x28/0x20c
>>>> dw_mci_read_data_pio+0x44/0x190
>>>> dw_mmc f000a000.mmc: Unexpected interrupt latency
>>
>> I think we tested SD cards but the main reason we missed
>> this is that we don't use pio mode since dw_mmc decides
>> the transfer mode via HCON register but we don't have one
>> platform at hand then to do that. Given the data-transfer-over
>> interrupt should come after the data hit the RAM, so pio mode
>> could probably consume more time than IDMAC.
>
> That's really interesting.
>
> I was under impression that we use internal DMA controller (AKA IDMAC)
> on that platform (HSDK).
>
> This is what we typically see in bootlog (this extract is taken from
> 4.15-r9):
> --------------------------------->8--------------------------------
> dw_mmc f000a000.mmc: 'num-slots' was deprecated.
> dw_mmc f000a000.mmc: IDMAC supports 32-bit address mode.
> dw_mmc f000a000.mmc: Using internal DMA controller.
> dw_mmc f000a000.mmc: Version ID is 290a
> dw_mmc f000a000.mmc: DW MMC controller at irq 12,32 bit host data width,16 deep fifo
> --------------------------------->8--------------------------------
>
> I'm not really sure how PIO mode (which stands for non-DMA mode) got used
> given we have IDMAC in the hardware.
There is a DW_MCI_DMA_THRESHOLD to decides whether we use IDMA or pio
per request, since pio probably do fast than IDMA which need setup the
decriptor list if the request size is small. But that never happen for
this case as the V1 commit log said "copying file from mmc" which means
your minimal data block size is 512 anyway. So the only reasonable guess
is that just happend when initializing the card rather then copying the
file..
>
> @ Evgeniy, could you please check why IDMAC is not used?
>
> -Alexey
>
--
Best Regards
Shawn Lin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
2018-02-22 15:28 ` Shawn Lin
2018-02-22 16:03 ` Alexey Brodkin
@ 2018-02-22 17:38 ` Vineet Gupta
1 sibling, 0 replies; 6+ messages in thread
From: Vineet Gupta @ 2018-02-22 17:38 UTC (permalink / raw)
To: linux-snps-arc
On 02/22/2018 07:28 AM, Shawn Lin wrote:
>>
>> Lets cast this multiply to u64 type which prevents overflow.
>>
>> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
>> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
>>
>> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>>
>> CC: Alexey Brodkin <abrodkin at synopsys.com>
>> CC: Eugeniy Paltsev <paltsev at synopsys.com>
>> CC: Douglas Anderson <dianders at chromium.org>
>> CC: Ulf Hansson <ulf.hansson at linaro.org>
>> CC: linux-kernel at vger.kernel.org
>> CC: linux-snps-arc at lists.infradead.org
>> Cc: <stable at vger.kernel.org> #? 9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout
>> calculation
>
> The fix looks good to me, but the tags should be improved a bit:
>
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
Ok, then perhaps use Reported-by tag to call out when/how issue was found.
Reported-by: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> Cc: <stable at vger.kernel.org>
>
> Otherwise,
> Reviewed-by: Shawn Lin <shawn.lin at rock-chips.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
2018-02-22 13:34 [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems Evgeniy Didin
2018-02-22 15:28 ` Shawn Lin
@ 2018-02-23 3:24 ` Jisheng Zhang
1 sibling, 0 replies; 6+ messages in thread
From: Jisheng Zhang @ 2018-02-23 3:24 UTC (permalink / raw)
To: linux-snps-arc
On Thu, 22 Feb 2018 16:34:18 +0300 Evgeniy Didin wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
> changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations caused false interrupt latency warnings
> and stacktrace splat (such as below) when accessing the SD Card.
>
> | Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
> | - Info: Finished target initialization.
> | mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
> | 0x900, card status 0x0
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
> | 396825HZ div = 63)
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
> | 25000000HZ div = 1)
> | ------------[ cut here ]------------
> | softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
> | exited with 00000100?
> | WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
> | Modules linked in:
> | CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
> |
> | Stack Trace:
> | arc_unwind_core.constprop.1+0xd0/0xf4
> | dump_stack+0x68/0x80
> | warn_slowpath_null+0x4e/0xec
> | sg_miter_next+0x28/0x20c
> | dw_mci_read_data_pio+0x44/0x190
> | dw_mmc f000a000.mmc: Unexpected interrupt latency
> | dw_mci_interrupt+0x3ee/0x530
> | __handle_irq_event_percpu+0x56/0x150
> | handle_irq_event+0x34/0x78
> | handle_level_irq+0x8e/0x120
> | generic_handle_irq+0x1c/0x2c
> | idu_cascade_isr+0x30/0x6c
> | __handle_domain_irq+0x72/0xc8
> | ret_from_exception+0x0/0x8
> |---[ end trace 2a58c9af6c25fe51 ]---
>
> Lets cast this multiply to u64 type which prevents overflow.
>
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
>
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>
> CC: Alexey Brodkin <abrodkin at synopsys.com>
> CC: Eugeniy Paltsev <paltsev at synopsys.com>
> CC: Douglas Anderson <dianders at chromium.org>
> CC: Ulf Hansson <ulf.hansson at linaro.org>
> CC: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org> # 9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
> ---
> Changes since v1:
> -uint64_t switched to u64
> -changed commit message
> drivers/mmc/host/dw_mmc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..1a0b9751c67c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1944,7 +1944,7 @@ static void dw_mci_set_drto(struct dw_mci *host)
> drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (drto_div == 0)
> drto_div = 1;
> - drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
> + drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
> host->bus_hz);
DIV_ROUND_UP_ULL?
>
> /* add a bit spare time */
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-23 3:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-22 13:34 [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems Evgeniy Didin
2018-02-22 15:28 ` Shawn Lin
2018-02-22 16:03 ` Alexey Brodkin
2018-02-22 16:36 ` Shawn Lin
2018-02-22 17:38 ` Vineet Gupta
2018-02-23 3:24 ` Jisheng Zhang
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).