diff mbox series

[v2,1/1] mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard

Message ID 20240603220834.21989-2-kamal.dasu@broadcom.com
State New
Headers show
Series mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard | expand

Commit Message

Kamal Dasu June 3, 2024, 10:08 p.m. UTC 
When erase/trim/discard completion was converted to mmc_poll_for_busy(),
optional ->card_busy() host ops support was added as part of
dd0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for erase/trim/discard").
sdhci card->busy() could return busy for long periods to cause
mmc_do_erase() to block during discard operation as shown below
during mkfs.f2fs :

    Info: [/dev/mmcblk1p9] Discarding device
    [   39.597258] sysrq: Show Blocked State
    [   39.601183] task:mkfs.f2fs       state:D stack:0     pid:1561  tgid:1561  ppid:1542   flags:0x0000000d
    [   39.610609] Call trace:
    [   39.613098]  __switch_to+0xd8/0xf4
    [   39.616582]  __schedule+0x440/0x4f4
    [   39.620137]  schedule+0x2c/0x48
    [   39.623341]  schedule_hrtimeout_range_clock+0xe0/0x114
    [   39.628562]  schedule_hrtimeout_range+0x10/0x18
    [   39.633169]  usleep_range_state+0x5c/0x90
    [   39.637253]  __mmc_poll_for_busy+0xec/0x128
    [   39.641514]  mmc_poll_for_busy+0x48/0x70
    [   39.645511]  mmc_do_erase+0x1ec/0x210
    [   39.649237]  mmc_erase+0x1b4/0x1d4
    [   39.652701]  mmc_blk_mq_issue_rq+0x35c/0x6ac
    [   39.657037]  mmc_mq_queue_rq+0x18c/0x214
    [   39.661022]  blk_mq_dispatch_rq_list+0x3a8/0x528
    [   39.665722]  __blk_mq_sched_dispatch_requests+0x3a0/0x4ac
    [   39.671198]  blk_mq_sched_dispatch_requests+0x28/0x5c
    [   39.676322]  blk_mq_run_hw_queue+0x11c/0x12c
    [   39.680668]  blk_mq_flush_plug_list+0x200/0x33c
    [   39.685278]  blk_add_rq_to_plug+0x68/0xd8
    [   39.689365]  blk_mq_submit_bio+0x3a4/0x458
    [   39.693539]  __submit_bio+0x1c/0x80
    [   39.697096]  submit_bio_noacct_nocheck+0x94/0x174
    [   39.701875]  submit_bio_noacct+0x1b0/0x22c
    [   39.706042]  submit_bio+0xac/0xe8
    [   39.709424]  blk_next_bio+0x4c/0x5c
    [   39.712973]  blkdev_issue_secure_erase+0x118/0x170
    [   39.717835]  blkdev_common_ioctl+0x374/0x728
    [   39.722175]  blkdev_ioctl+0x8c/0x2b0
    [   39.725816]  vfs_ioctl+0x24/0x40
    [   39.729117]  __arm64_sys_ioctl+0x5c/0x8c
    [   39.733114]  invoke_syscall+0x68/0xec
    [   39.736839]  el0_svc_common.constprop.0+0x70/0xd8
    [   39.741609]  do_el0_svc+0x18/0x20
    [   39.744981]  el0_svc+0x68/0x94
    [   39.748107]  el0t_64_sync_handler+0x88/0x124
    [   39.752455]  el0t_64_sync+0x168/0x16c

This problem is obsereved with BLKSECDISCARD ioctl on brcmstb mmc
controllers. Fix makes mmc_host_ops.card_busy NULL and forces
MMC_SEND_STATUS and R1_STATUS check in mmc_busy_cb() function.

Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
---
 drivers/mmc/host/sdhci-brcmstb.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Ulf Hansson June 4, 2024, 11:13 a.m. UTC | #1 
On Tue, 4 Jun 2024 at 00:09, Kamal Dasu <kamal.dasu@broadcom.com> wrote:
>
> When erase/trim/discard completion was converted to mmc_poll_for_busy(),
> optional ->card_busy() host ops support was added as part of
> dd0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for erase/trim/discard").

I can't find the above commit hash. You probably want "0d84c3e6a5b2"?

> sdhci card->busy() could return busy for long periods to cause
> mmc_do_erase() to block during discard operation as shown below
> during mkfs.f2fs :
>
>     Info: [/dev/mmcblk1p9] Discarding device
>     [   39.597258] sysrq: Show Blocked State
>     [   39.601183] task:mkfs.f2fs       state:D stack:0     pid:1561  tgid:1561  ppid:1542   flags:0x0000000d
>     [   39.610609] Call trace:
>     [   39.613098]  __switch_to+0xd8/0xf4
>     [   39.616582]  __schedule+0x440/0x4f4
>     [   39.620137]  schedule+0x2c/0x48
>     [   39.623341]  schedule_hrtimeout_range_clock+0xe0/0x114
>     [   39.628562]  schedule_hrtimeout_range+0x10/0x18
>     [   39.633169]  usleep_range_state+0x5c/0x90
>     [   39.637253]  __mmc_poll_for_busy+0xec/0x128
>     [   39.641514]  mmc_poll_for_busy+0x48/0x70
>     [   39.645511]  mmc_do_erase+0x1ec/0x210
>     [   39.649237]  mmc_erase+0x1b4/0x1d4
>     [   39.652701]  mmc_blk_mq_issue_rq+0x35c/0x6ac
>     [   39.657037]  mmc_mq_queue_rq+0x18c/0x214
>     [   39.661022]  blk_mq_dispatch_rq_list+0x3a8/0x528
>     [   39.665722]  __blk_mq_sched_dispatch_requests+0x3a0/0x4ac
>     [   39.671198]  blk_mq_sched_dispatch_requests+0x28/0x5c
>     [   39.676322]  blk_mq_run_hw_queue+0x11c/0x12c
>     [   39.680668]  blk_mq_flush_plug_list+0x200/0x33c
>     [   39.685278]  blk_add_rq_to_plug+0x68/0xd8
>     [   39.689365]  blk_mq_submit_bio+0x3a4/0x458
>     [   39.693539]  __submit_bio+0x1c/0x80
>     [   39.697096]  submit_bio_noacct_nocheck+0x94/0x174
>     [   39.701875]  submit_bio_noacct+0x1b0/0x22c
>     [   39.706042]  submit_bio+0xac/0xe8
>     [   39.709424]  blk_next_bio+0x4c/0x5c
>     [   39.712973]  blkdev_issue_secure_erase+0x118/0x170
>     [   39.717835]  blkdev_common_ioctl+0x374/0x728
>     [   39.722175]  blkdev_ioctl+0x8c/0x2b0
>     [   39.725816]  vfs_ioctl+0x24/0x40
>     [   39.729117]  __arm64_sys_ioctl+0x5c/0x8c
>     [   39.733114]  invoke_syscall+0x68/0xec
>     [   39.736839]  el0_svc_common.constprop.0+0x70/0xd8
>     [   39.741609]  do_el0_svc+0x18/0x20
>     [   39.744981]  el0_svc+0x68/0x94
>     [   39.748107]  el0t_64_sync_handler+0x88/0x124
>     [   39.752455]  el0t_64_sync+0x168/0x16c
>
> This problem is obsereved with BLKSECDISCARD ioctl on brcmstb mmc
> controllers. Fix makes mmc_host_ops.card_busy NULL and forces
> MMC_SEND_STATUS and R1_STATUS check in mmc_busy_cb() function.
>
> Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>

We probably want a fixes/stable tag for this too, right?

Fixes: 0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for
erase/trim/discard")

I have amended the commit message and applied this for fixes, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-brcmstb.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> index 9053526fa212..150fb477b7cc 100644
> --- a/drivers/mmc/host/sdhci-brcmstb.c
> +++ b/drivers/mmc/host/sdhci-brcmstb.c
> @@ -24,6 +24,7 @@
>  #define BRCMSTB_MATCH_FLAGS_NO_64BIT           BIT(0)
>  #define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT     BIT(1)
>  #define BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE     BIT(2)
> +#define BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY      BIT(4)
>
>  #define BRCMSTB_PRIV_FLAGS_HAS_CQE             BIT(0)
>  #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK          BIT(1)
> @@ -384,6 +385,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
>         if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
>                 host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> +       if (!(match_priv->flags & BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY))
> +               host->mmc_host_ops.card_busy = NULL;
> +
>         /* Change the base clock frequency if the DT property exists */
>         if (device_property_read_u32(&pdev->dev, "clock-frequency",
>                                      &priv->base_freq_hz) != 0)
> --
> 2.17.1
>
Kamal Dasu June 4, 2024, 3:42 p.m. UTC | #2 
On Tue, Jun 4, 2024 at 7:14 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Tue, 4 Jun 2024 at 00:09, Kamal Dasu <kamal.dasu@broadcom.com> wrote:
> >
> > When erase/trim/discard completion was converted to mmc_poll_for_busy(),
> > optional ->card_busy() host ops support was added as part of
> > dd0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for erase/trim/discard").
>
> I can't find the above commit hash. You probably want "0d84c3e6a5b2"?
>
> > sdhci card->busy() could return busy for long periods to cause
> > mmc_do_erase() to block during discard operation as shown below
> > during mkfs.f2fs :
> >
> >     Info: [/dev/mmcblk1p9] Discarding device
> >     [   39.597258] sysrq: Show Blocked State
> >     [   39.601183] task:mkfs.f2fs       state:D stack:0     pid:1561  tgid:1561  ppid:1542   flags:0x0000000d
> >     [   39.610609] Call trace:
> >     [   39.613098]  __switch_to+0xd8/0xf4
> >     [   39.616582]  __schedule+0x440/0x4f4
> >     [   39.620137]  schedule+0x2c/0x48
> >     [   39.623341]  schedule_hrtimeout_range_clock+0xe0/0x114
> >     [   39.628562]  schedule_hrtimeout_range+0x10/0x18
> >     [   39.633169]  usleep_range_state+0x5c/0x90
> >     [   39.637253]  __mmc_poll_for_busy+0xec/0x128
> >     [   39.641514]  mmc_poll_for_busy+0x48/0x70
> >     [   39.645511]  mmc_do_erase+0x1ec/0x210
> >     [   39.649237]  mmc_erase+0x1b4/0x1d4
> >     [   39.652701]  mmc_blk_mq_issue_rq+0x35c/0x6ac
> >     [   39.657037]  mmc_mq_queue_rq+0x18c/0x214
> >     [   39.661022]  blk_mq_dispatch_rq_list+0x3a8/0x528
> >     [   39.665722]  __blk_mq_sched_dispatch_requests+0x3a0/0x4ac
> >     [   39.671198]  blk_mq_sched_dispatch_requests+0x28/0x5c
> >     [   39.676322]  blk_mq_run_hw_queue+0x11c/0x12c
> >     [   39.680668]  blk_mq_flush_plug_list+0x200/0x33c
> >     [   39.685278]  blk_add_rq_to_plug+0x68/0xd8
> >     [   39.689365]  blk_mq_submit_bio+0x3a4/0x458
> >     [   39.693539]  __submit_bio+0x1c/0x80
> >     [   39.697096]  submit_bio_noacct_nocheck+0x94/0x174
> >     [   39.701875]  submit_bio_noacct+0x1b0/0x22c
> >     [   39.706042]  submit_bio+0xac/0xe8
> >     [   39.709424]  blk_next_bio+0x4c/0x5c
> >     [   39.712973]  blkdev_issue_secure_erase+0x118/0x170
> >     [   39.717835]  blkdev_common_ioctl+0x374/0x728
> >     [   39.722175]  blkdev_ioctl+0x8c/0x2b0
> >     [   39.725816]  vfs_ioctl+0x24/0x40
> >     [   39.729117]  __arm64_sys_ioctl+0x5c/0x8c
> >     [   39.733114]  invoke_syscall+0x68/0xec
> >     [   39.736839]  el0_svc_common.constprop.0+0x70/0xd8
> >     [   39.741609]  do_el0_svc+0x18/0x20
> >     [   39.744981]  el0_svc+0x68/0x94
> >     [   39.748107]  el0t_64_sync_handler+0x88/0x124
> >     [   39.752455]  el0t_64_sync+0x168/0x16c
> >
> > This problem is obsereved with BLKSECDISCARD ioctl on brcmstb mmc
> > controllers. Fix makes mmc_host_ops.card_busy NULL and forces
> > MMC_SEND_STATUS and R1_STATUS check in mmc_busy_cb() function.
> >
> > Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
>
> We probably want a fixes/stable tag for this too, right?
>
> Fixes: 0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for
> erase/trim/discard")
>
> I have amended the commit message and applied this for fixes, thanks!
>

Thank you Ulffe.

> Kind regards
> Uffe
>
>
> > ---
> >  drivers/mmc/host/sdhci-brcmstb.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> > index 9053526fa212..150fb477b7cc 100644
> > --- a/drivers/mmc/host/sdhci-brcmstb.c
> > +++ b/drivers/mmc/host/sdhci-brcmstb.c
> > @@ -24,6 +24,7 @@
> >  #define BRCMSTB_MATCH_FLAGS_NO_64BIT           BIT(0)
> >  #define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT     BIT(1)
> >  #define BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE     BIT(2)
> > +#define BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY      BIT(4)
> >
> >  #define BRCMSTB_PRIV_FLAGS_HAS_CQE             BIT(0)
> >  #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK          BIT(1)
> > @@ -384,6 +385,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> >         if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
> >                 host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> >
> > +       if (!(match_priv->flags & BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY))
> > +               host->mmc_host_ops.card_busy = NULL;
> > +
> >         /* Change the base clock frequency if the DT property exists */
> >         if (device_property_read_u32(&pdev->dev, "clock-frequency",
> >                                      &priv->base_freq_hz) != 0)
> > --
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
index 9053526fa212..150fb477b7cc 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -24,6 +24,7 @@ 
 #define BRCMSTB_MATCH_FLAGS_NO_64BIT		BIT(0)
 #define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT	BIT(1)
 #define BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE	BIT(2)
+#define BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY	BIT(4)
 
 #define BRCMSTB_PRIV_FLAGS_HAS_CQE		BIT(0)
 #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK		BIT(1)
@@ -384,6 +385,9 @@  static int sdhci_brcmstb_probe(struct platform_device *pdev)
 	if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
+	if (!(match_priv->flags & BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY))
+		host->mmc_host_ops.card_busy = NULL;
+
 	/* Change the base clock frequency if the DT property exists */
 	if (device_property_read_u32(&pdev->dev, "clock-frequency",
 				     &priv->base_freq_hz) != 0)