| Message ID | 1603883984-24333-1-git-send-email-vbadigan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: block: Prevent new req entering queue while freeing up the queue | expand |
Hi Ulf, Adrian, Gentle reminder. Can you share your comments on this issue and change? Thanks On 10/28/2020 4:49 PM, Veerabhadrarao Badiganti wrote: > The commit bbdc74dc19e0 ("mmc: block: Prevent new req entering queue > after its cleanup") has introduced this change but it got moved after > del_gendisk() with commit 57678e5a3d51 ("mmc: block: Delete gendisk > before cleaning up the request queue"). > > It is blocking reboot with below Call stack(). > > INFO: task reboot:3086 blocked for more than 122 seconds. > __schedule > schedule > schedule_timeout > io_schedule_timeout > do_wait_for_common > wait_for_completion_io > submit_bio_wait > blkdev_issue_flush > ext4_sync_fs > __sync_filesystem > sync_filesystem > fsync_bdev > invalidate_partition > del_gendisk > mmc_blk_remove_req > mmc_blk_remove > mmc_bus_remove > device_release_driver_internal > device_release_driver > bus_remove_device > device_del > mmc_remove_card > mmc_remove > mmc_stop_host > mmc_remove_host > sdhci_remove_host > sdhci_msm_remove > sdhci_msm_shutdown > platform_drv_shutdown > device_shutdown > kernel_restart_prepare > kernel_restart > > So bringing this change back. > > Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> > --- > > I'm observing this issue 100% of the time with shutdown callback added to sdhci-msm driver. > I'm trying on 5.4 kernel with ChromeOS. > > Please let me know if this can be fixed in a better way. > --- > > drivers/mmc/core/block.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index 8d3df0be0355..76dbb2b8a13b 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -2627,6 +2627,7 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md) > * from being accepted. > */ > card = md->queue.card; > + blk_set_queue_dying(md->queue.queue); > if (md->disk->flags & GENHD_FL_UP) { > device_remove_file(disk_to_dev(md->disk), &md->force_ro); > if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
On Wed, 28 Oct 2020 at 12:20, Veerabhadrarao Badiganti <vbadigan@codeaurora.org> wrote: > > The commit bbdc74dc19e0 ("mmc: block: Prevent new req entering queue > after its cleanup") has introduced this change but it got moved after > del_gendisk() with commit 57678e5a3d51 ("mmc: block: Delete gendisk > before cleaning up the request queue"). This isn't the first time we have spotted errors in this path. Seems like a difficult path to get correct. :-) > > It is blocking reboot with below Call stack(). > > INFO: task reboot:3086 blocked for more than 122 seconds. > __schedule > schedule > schedule_timeout > io_schedule_timeout > do_wait_for_common > wait_for_completion_io > submit_bio_wait > blkdev_issue_flush > ext4_sync_fs > __sync_filesystem > sync_filesystem > fsync_bdev > invalidate_partition > del_gendisk > mmc_blk_remove_req > mmc_blk_remove > mmc_bus_remove > device_release_driver_internal > device_release_driver > bus_remove_device > device_del > mmc_remove_card > mmc_remove > mmc_stop_host > mmc_remove_host > sdhci_remove_host > sdhci_msm_remove Why do you call sdhci_msm_remove() from the shutdown callback? What specific operations do you need to run in the shutdown path for sdhci msm? The important part should be to do a graceful shutdown of the card (and the block device) - is there anything else? Or you are just using the shutdown callback as a simple way to trigger this problem? Could unbinding the driver trigger the same issue? > sdhci_msm_shutdown > platform_drv_shutdown > device_shutdown > kernel_restart_prepare > kernel_restart > > So bringing this change back. > > Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> > --- > > I'm observing this issue 100% of the time with shutdown callback added to sdhci-msm driver. > I'm trying on 5.4 kernel with ChromeOS. > > Please let me know if this can be fixed in a better way. I don't know yet, but I will have a closer look. Let's also see if Adrian has some thoughts. Kind regards Uffe > --- > > drivers/mmc/core/block.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index 8d3df0be0355..76dbb2b8a13b 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -2627,6 +2627,7 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md) > * from being accepted. > */ > card = md->queue.card; > + blk_set_queue_dying(md->queue.queue); > if (md->disk->flags & GENHD_FL_UP) { > device_remove_file(disk_to_dev(md->disk), &md->force_ro); > if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) && > -- > Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project >
On 3/11/20 5:25 pm, Ulf Hansson wrote: > On Wed, 28 Oct 2020 at 12:20, Veerabhadrarao Badiganti > <vbadigan@codeaurora.org> wrote: >> >> The commit bbdc74dc19e0 ("mmc: block: Prevent new req entering queue >> after its cleanup") has introduced this change but it got moved after >> del_gendisk() with commit 57678e5a3d51 ("mmc: block: Delete gendisk >> before cleaning up the request queue"). > > This isn't the first time we have spotted errors in this path. Seems > like a difficult path to get correct. :-) > >> >> It is blocking reboot with below Call stack(). >> >> INFO: task reboot:3086 blocked for more than 122 seconds. >> __schedule >> schedule >> schedule_timeout >> io_schedule_timeout >> do_wait_for_common >> wait_for_completion_io >> submit_bio_wait >> blkdev_issue_flush >> ext4_sync_fs >> __sync_filesystem >> sync_filesystem >> fsync_bdev >> invalidate_partition >> del_gendisk >> mmc_blk_remove_req >> mmc_blk_remove >> mmc_bus_remove >> device_release_driver_internal >> device_release_driver >> bus_remove_device >> device_del >> mmc_remove_card >> mmc_remove >> mmc_stop_host >> mmc_remove_host >> sdhci_remove_host >> sdhci_msm_remove > > Why do you call sdhci_msm_remove() from the shutdown callback? What > specific operations do you need to run in the shutdown path for sdhci > msm? Yes, the problem is that upper layers, like the mmc block driver, have already shut down, so doing operations like remove will get into deadlocks. > > The important part should be to do a graceful shutdown of the card > (and the block device) - is there anything else? > > Or you are just using the shutdown callback as a simple way to trigger > this problem? Could unbinding the driver trigger the same issue? > >> sdhci_msm_shutdown >> platform_drv_shutdown >> device_shutdown >> kernel_restart_prepare >> kernel_restart >> >> So bringing this change back. >> >> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> >> --- >> >> I'm observing this issue 100% of the time with shutdown callback added to sdhci-msm driver. >> I'm trying on 5.4 kernel with ChromeOS. >> >> Please let me know if this can be fixed in a better way. > > I don't know yet, but I will have a closer look. Let's also see if > Adrian has some thoughts. > > Kind regards > Uffe > >> --- >> >> drivers/mmc/core/block.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c >> index 8d3df0be0355..76dbb2b8a13b 100644 >> --- a/drivers/mmc/core/block.c >> +++ b/drivers/mmc/core/block.c >> @@ -2627,6 +2627,7 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md) >> * from being accepted. >> */ >> card = md->queue.card; >> + blk_set_queue_dying(md->queue.queue); >> if (md->disk->flags & GENHD_FL_UP) { >> device_remove_file(disk_to_dev(md->disk), &md->force_ro); >> if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) && >> -- >> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project >>
On 11/3/2020 8:55 PM, Ulf Hansson wrote: > On Wed, 28 Oct 2020 at 12:20, Veerabhadrarao Badiganti > <vbadigan@codeaurora.org> wrote: >> The commit bbdc74dc19e0 ("mmc: block: Prevent new req entering queue >> after its cleanup") has introduced this change but it got moved after >> del_gendisk() with commit 57678e5a3d51 ("mmc: block: Delete gendisk >> before cleaning up the request queue"). > This isn't the first time we have spotted errors in this path. Seems > like a difficult path to get correct. :-) > >> It is blocking reboot with below Call stack(). >> >> INFO: task reboot:3086 blocked for more than 122 seconds. >> __schedule >> schedule >> schedule_timeout >> io_schedule_timeout >> do_wait_for_common >> wait_for_completion_io >> submit_bio_wait >> blkdev_issue_flush >> ext4_sync_fs >> __sync_filesystem >> sync_filesystem >> fsync_bdev >> invalidate_partition >> del_gendisk >> mmc_blk_remove_req >> mmc_blk_remove >> mmc_bus_remove >> device_release_driver_internal >> device_release_driver >> bus_remove_device >> device_del >> mmc_remove_card >> mmc_remove >> mmc_stop_host >> mmc_remove_host >> sdhci_remove_host >> sdhci_msm_remove > Why do you call sdhci_msm_remove() from the shutdown callback? What > specific operations do you need to run in the shutdown path for sdhci > msm? I was suggested to add shutdown callback by memory team to gracefully de-register with smmu driver during reboot/shutdown. So tried adding it. Since SMMU team. I just need to ensure that controller is not active anymore and not accessing memory related to any requests and de-register with smmu driver during shutdown. > > The important part should be to do a graceful shutdown of the card > (and the block device) - is there anything else? > > Or you are just using the shutdown callback as a simple way to trigger > this problem? Could unbinding the driver trigger the same issue? > >> sdhci_msm_shutdown >> platform_drv_shutdown >> device_shutdown >> kernel_restart_prepare >> kernel_restart >> >> So bringing this change back. >> >> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> >> --- >> >> I'm observing this issue 100% of the time with shutdown callback added to sdhci-msm driver. >> I'm trying on 5.4 kernel with ChromeOS. >> >> Please let me know if this can be fixed in a better way. > I don't know yet, but I will have a closer look. Let's also see if > Adrian has some thoughts. > > Kind regards > Uffe > >> --- >> >> drivers/mmc/core/block.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c >> index 8d3df0be0355..76dbb2b8a13b 100644 >> --- a/drivers/mmc/core/block.c >> +++ b/drivers/mmc/core/block.c >> @@ -2627,6 +2627,7 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md) >> * from being accepted. >> */ >> card = md->queue.card; >> + blk_set_queue_dying(md->queue.queue); >> if (md->disk->flags & GENHD_FL_UP) { >> device_remove_file(disk_to_dev(md->disk), &md->force_ro); >> if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) && >> -- >> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project >>
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 8d3df0be0355..76dbb2b8a13b 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -2627,6 +2627,7 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md) * from being accepted. */ card = md->queue.card; + blk_set_queue_dying(md->queue.queue); if (md->disk->flags & GENHD_FL_UP) { device_remove_file(disk_to_dev(md->disk), &md->force_ro); if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
The commit bbdc74dc19e0 ("mmc: block: Prevent new req entering queue after its cleanup") has introduced this change but it got moved after del_gendisk() with commit 57678e5a3d51 ("mmc: block: Delete gendisk before cleaning up the request queue"). It is blocking reboot with below Call stack(). INFO: task reboot:3086 blocked for more than 122 seconds. __schedule schedule schedule_timeout io_schedule_timeout do_wait_for_common wait_for_completion_io submit_bio_wait blkdev_issue_flush ext4_sync_fs __sync_filesystem sync_filesystem fsync_bdev invalidate_partition del_gendisk mmc_blk_remove_req mmc_blk_remove mmc_bus_remove device_release_driver_internal device_release_driver bus_remove_device device_del mmc_remove_card mmc_remove mmc_stop_host mmc_remove_host sdhci_remove_host sdhci_msm_remove sdhci_msm_shutdown platform_drv_shutdown device_shutdown kernel_restart_prepare kernel_restart So bringing this change back. Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org> --- I'm observing this issue 100% of the time with shutdown callback added to sdhci-msm driver. I'm trying on 5.4 kernel with ChromeOS. Please let me know if this can be fixed in a better way. --- drivers/mmc/core/block.c | 1 + 1 file changed, 1 insertion(+)