| Message ID | 1461868156-5734-1-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 29 April 2016 at 20:40, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 28/04/2016 9:29 p.m., Ulf Hansson wrote: >> >> For the reasons explained below, it's safe to remove the rescan_disable >> flag. >> >> 1) >> cancel_delayed_work_sync() prevents an executing work from re-scheduling >> itself. >> >> 2) >> We are using the system_freezable_wq for the rescan works. As that queue >> becomes frozen when userspace becomes frozen during system PM, we don't >> need to disable the rescan works in the PM_SUSPEND_PREPARE phase. > > > What about the case when a SDIO card has to be removed? Seems like a rescan > could theoretically get scheduled and run before the workqueue is frozen. When the queue gets frozen, it means currently running works will be synced. Works that are scheduled (or becomes scheduled) will be put on hold and not allowed to run before the workqueue becomes un-frozen. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10 May 2016 at 11:07, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 10/05/16 12:01, Ulf Hansson wrote: >> On 29 April 2016 at 20:40, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> On 28/04/2016 9:29 p.m., Ulf Hansson wrote: >>>> >>>> For the reasons explained below, it's safe to remove the rescan_disable >>>> flag. >>>> >>>> 1) >>>> cancel_delayed_work_sync() prevents an executing work from re-scheduling >>>> itself. >>>> >>>> 2) >>>> We are using the system_freezable_wq for the rescan works. As that queue >>>> becomes frozen when userspace becomes frozen during system PM, we don't >>>> need to disable the rescan works in the PM_SUSPEND_PREPARE phase. >>> >>> >>> What about the case when a SDIO card has to be removed? Seems like a rescan >>> could theoretically get scheduled and run before the workqueue is frozen. >> >> When the queue gets frozen, it means currently running works will be >> synced. Works that are scheduled (or becomes scheduled) will be put on >> hold and not allowed to run before the workqueue becomes un-frozen. > > In the pm_notifier the workqueue is not frozen, so new work could be queued > and run, which would potentially race with the "host->bus_ops->remove(host)" > a few lines further on. You are right, I will put this patch on hold! Thanks! I need to change the validation for this special SDIO case first. We shouldn't need to detach the bus and thus also remove the card in these cases, but instead only remove the SDIO func devices. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 99275e4..b8c6a11 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2580,9 +2580,6 @@ void mmc_rescan(struct work_struct *work) container_of(work, struct mmc_host, detect.work); int i; - if (host->rescan_disable) - return; - /* If there is a non-removable card registered, only scan once */ if (!mmc_card_is_removable(host) && host->rescan_entered) return; @@ -2649,7 +2646,6 @@ void mmc_rescan(struct work_struct *work) void mmc_start_host(struct mmc_host *host) { host->f_init = max(freqs[0], host->f_min); - host->rescan_disable = 0; host->ios.power_mode = MMC_POWER_UNDEFINED; mmc_claim_host(host); @@ -2674,7 +2670,6 @@ void mmc_stop_host(struct mmc_host *host) if (host->slot.cd_irq >= 0) disable_irq(host->slot.cd_irq); - host->rescan_disable = 1; cancel_delayed_work_sync(&host->detect); /* clear pm flags now and let card drivers set them as needed */ @@ -2788,9 +2783,6 @@ static int mmc_pm_notify(struct notifier_block *notify_block, case PM_HIBERNATION_PREPARE: case PM_SUSPEND_PREPARE: case PM_RESTORE_PREPARE: - spin_lock_irqsave(&host->lock, flags); - host->rescan_disable = 1; - spin_unlock_irqrestore(&host->lock, flags); cancel_delayed_work_sync(&host->detect); if (!host->bus_ops) @@ -2814,10 +2806,6 @@ static int mmc_pm_notify(struct notifier_block *notify_block, case PM_POST_SUSPEND: case PM_POST_HIBERNATION: case PM_POST_RESTORE: - - spin_lock_irqsave(&host->lock, flags); - host->rescan_disable = 0; - spin_unlock_irqrestore(&host->lock, flags); _mmc_detect_change(host, 0, false); } diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index e0a3ee1..e972489 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -319,9 +319,6 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) if (!host) return NULL; - /* scanning will be enabled when we're ready */ - host->rescan_disable = 1; - again: if (!ida_pre_get(&mmc_host_ida, GFP_KERNEL)) { kfree(host); diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 85800b4..7fbe5c0 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -330,7 +330,6 @@ struct mmc_host { unsigned int doing_retune:1; /* re-tuning in progress */ unsigned int retune_now:1; /* do re-tuning at next req */ - int rescan_disable; /* disable card detection */ int rescan_entered; /* used with nonremovable devices */ int need_retune; /* re-tuning is needed */
For the reasons explained below, it's safe to remove the rescan_disable flag. 1) cancel_delayed_work_sync() prevents an executing work from re-scheduling itself. 2) We are using the system_freezable_wq for the rescan works. As that queue becomes frozen when userspace becomes frozen during system PM, we don't need to disable the rescan works in the PM_SUSPEND_PREPARE phase. 3) At host registration it's not safe to schedule a detect work until mmc_start_host() is invoked. Trying to use the rescan_disable flag to prevent a rescan work from being executed isn't helping. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- Changes in v2: Updated changelog to better reflect why rescan_flag is redundant. --- drivers/mmc/core/core.c | 12 ------------ drivers/mmc/core/host.c | 3 --- include/linux/mmc/host.h | 1 - 3 files changed, 16 deletions(-) -- 1.9.1