From: Maxim Levitsky <maximlevitsky@gmail.com>
To: Ohad Ben-Cohen <ohad@wizery.com>
Cc: linux-mmc@vger.kernel.org, Sven Neumann <s.neumann@raumfeld.com>,
Chris Ball <cjb@laptop.org>, Nicolas Pitre <nico@fluxnic.net>,
libertas-dev@lists.infradead.org,
"Rafael J. Wysocki" <rjw@sisk.pl>, Daniel Mack <daniel@caiaq.de>,
Colin Cross <ccross@android.com>,
Greg Kroah-Hartman <gregkh@suse.de>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, stable@kernel.org
Subject: Re: [PATCH] sdio: fix suspend/resume regression
Date: Sat, 23 Oct 2010 16:18:50 +0200 [thread overview]
Message-ID: <1287843531.29752.10.camel@maxim-laptop> (raw)
In-Reply-To: <AANLkTi=iC1uKyToEXrroOj+JxQ2mW97WRVEXjv+9oQEa@mail.gmail.com>
On Sat, 2010-10-23 at 12:09 +0200, Ohad Ben-Cohen wrote:
> Hi Maxim,
>
> On Fri, Oct 22, 2010 at 1:47 AM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > On Wed, 2010-10-13 at 09:31 +0200, Ohad Ben-Cohen wrote:
> >> Fix SDIO suspend/resume regression introduced by
> >> 4c2ef25fe0b847d2ae818f74758ddb0be1c27d8e "mmc: fix all hangs related to
> >> mmc/sd card insert/removal during suspend/resume":
> ...
> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >> index c94565d..515ff39 100644
> >> --- a/drivers/mmc/core/core.c
> >> +++ b/drivers/mmc/core/core.c
> >> @@ -1682,6 +1682,19 @@ int mmc_suspend_host(struct mmc_host *host)
> >> if (host->bus_ops && !host->bus_dead) {
> >> if (host->bus_ops->suspend)
> >> err = host->bus_ops->suspend(host);
> >> + if (err == -ENOSYS || !host->bus_ops->resume) {
> > This reintroduces the bug I fixed.
> >
> > if the CONFIG_MMC_UNSAFE_RESUME isn't set (and that is default
> > unfortunately), the host->bus_ops->resume will be NULL (see core/mmc.c
> > mmc_ops), and therefore card will be removed, that will trigger a block
> > device removal, sync, and deadlock).
>
> Have you reproduced this or is it just a general concern ?
Only a general concern.
>
> I'm asking this because if CONFIG_MMC_UNSAFE_RESUME isn't set, then
> mmc_pm_notify() will remove the card and detach the bus.
>
> As a result, host->bus_ops will be unset, and host->bus_dead will be set.
>
> In this case, mmc_suspend_host() should skip the code you are concerned about.
You are right here.
There is a very unlikely case that suspend of sd/mmc card fails (with
CONFIG_MMC_UNSAFE_RESUME set, and that will trigger the hang, but that
currently isn't possible because both mmc_sd_suspend and mmc_suspend
just return 0 unconditionally).
Btw, two above functions are identical, so some refactoring won't hurt.
So sorry for noise (and bug).
Best regards,
Maxim Levitsky
WARNING: multiple messages have this Message-ID (diff)
From: maximlevitsky@gmail.com (Maxim Levitsky)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] sdio: fix suspend/resume regression
Date: Sat, 23 Oct 2010 16:18:50 +0200 [thread overview]
Message-ID: <1287843531.29752.10.camel@maxim-laptop> (raw)
In-Reply-To: <AANLkTi=iC1uKyToEXrroOj+JxQ2mW97WRVEXjv+9oQEa@mail.gmail.com>
On Sat, 2010-10-23 at 12:09 +0200, Ohad Ben-Cohen wrote:
> Hi Maxim,
>
> On Fri, Oct 22, 2010 at 1:47 AM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > On Wed, 2010-10-13 at 09:31 +0200, Ohad Ben-Cohen wrote:
> >> Fix SDIO suspend/resume regression introduced by
> >> 4c2ef25fe0b847d2ae818f74758ddb0be1c27d8e "mmc: fix all hangs related to
> >> mmc/sd card insert/removal during suspend/resume":
> ...
> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >> index c94565d..515ff39 100644
> >> --- a/drivers/mmc/core/core.c
> >> +++ b/drivers/mmc/core/core.c
> >> @@ -1682,6 +1682,19 @@ int mmc_suspend_host(struct mmc_host *host)
> >> if (host->bus_ops && !host->bus_dead) {
> >> if (host->bus_ops->suspend)
> >> err = host->bus_ops->suspend(host);
> >> + if (err == -ENOSYS || !host->bus_ops->resume) {
> > This reintroduces the bug I fixed.
> >
> > if the CONFIG_MMC_UNSAFE_RESUME isn't set (and that is default
> > unfortunately), the host->bus_ops->resume will be NULL (see core/mmc.c
> > mmc_ops), and therefore card will be removed, that will trigger a block
> > device removal, sync, and deadlock).
>
> Have you reproduced this or is it just a general concern ?
Only a general concern.
>
> I'm asking this because if CONFIG_MMC_UNSAFE_RESUME isn't set, then
> mmc_pm_notify() will remove the card and detach the bus.
>
> As a result, host->bus_ops will be unset, and host->bus_dead will be set.
>
> In this case, mmc_suspend_host() should skip the code you are concerned about.
You are right here.
There is a very unlikely case that suspend of sd/mmc card fails (with
CONFIG_MMC_UNSAFE_RESUME set, and that will trigger the hang, but that
currently isn't possible because both mmc_sd_suspend and mmc_suspend
just return 0 unconditionally).
Btw, two above functions are identical, so some refactoring won't hurt.
So sorry for noise (and bug).
Best regards,
Maxim Levitsky
next prev parent reply other threads:[~2010-10-23 14:18 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-04 7:30 2.6.35.6 fails to suspend (pxa2xx-mci.0) Sven Neumann
2010-10-04 7:30 ` Sven Neumann
2010-10-04 7:48 ` Eric Miao
2010-10-04 7:48 ` Eric Miao
2010-10-06 18:59 ` Maciej Rutecki
2010-10-06 18:59 ` Maciej Rutecki
2010-10-06 23:55 ` Daniel Mack
2010-10-06 23:55 ` Daniel Mack
2010-10-07 0:18 ` Rafael J. Wysocki
2010-10-07 0:18 ` Rafael J. Wysocki
2010-10-07 15:03 ` Sven Neumann
2010-10-07 15:03 ` Sven Neumann
2010-10-07 21:23 ` Rafael J. Wysocki
2010-10-07 21:23 ` Rafael J. Wysocki
2010-10-08 8:23 ` Sven Neumann
2010-10-08 8:23 ` Sven Neumann
2010-10-08 20:08 ` Rafael J. Wysocki
2010-10-08 20:08 ` Rafael J. Wysocki
2010-10-09 1:07 ` Ohad Ben-Cohen
2010-10-09 1:07 ` Ohad Ben-Cohen
2010-10-09 23:20 ` Sven Neumann
2010-10-09 23:20 ` Sven Neumann
2010-10-11 8:31 ` Sven Neumann
2010-10-11 8:31 ` Sven Neumann
2010-10-11 8:45 ` Ohad Ben-Cohen
2010-10-11 8:45 ` Ohad Ben-Cohen
2010-10-11 9:11 ` Sven Neumann
2010-10-11 9:11 ` Sven Neumann
2010-10-13 7:31 ` [PATCH] sdio: fix suspend/resume regression Ohad Ben-Cohen
2010-10-13 7:31 ` Ohad Ben-Cohen
2010-10-13 7:31 ` Ohad Ben-Cohen
2010-10-13 7:54 ` Vitaly Wool
2010-10-13 7:54 ` Vitaly Wool
2010-10-13 8:55 ` Ohad Ben-Cohen
2010-10-13 8:55 ` Ohad Ben-Cohen
2010-10-13 9:06 ` Vitaly Wool
2010-10-13 9:06 ` Vitaly Wool
2010-10-13 9:46 ` Ohad Ben-Cohen
2010-10-13 9:46 ` Ohad Ben-Cohen
2010-10-13 20:00 ` Nicolas Pitre
2010-10-13 20:00 ` Nicolas Pitre
2010-10-13 20:08 ` Nicolas Pitre
2010-10-13 20:08 ` Nicolas Pitre
2010-10-14 15:28 ` Ohad Ben-Cohen
2010-10-14 15:28 ` Ohad Ben-Cohen
2010-10-14 2:24 ` Chris Ball
2010-10-14 2:24 ` Chris Ball
2010-10-14 4:49 ` Ohad Ben-Cohen
2010-10-14 4:49 ` Ohad Ben-Cohen
2010-10-21 23:47 ` Maxim Levitsky
2010-10-21 23:47 ` Maxim Levitsky
2010-10-21 23:55 ` Nicolas Pitre
2010-10-21 23:55 ` Nicolas Pitre
2010-10-22 0:25 ` Maxim Levitsky
2010-10-22 0:25 ` Maxim Levitsky
2010-10-21 23:57 ` Nicolas Pitre
2010-10-21 23:57 ` Nicolas Pitre
2010-10-23 10:09 ` Ohad Ben-Cohen
2010-10-23 10:09 ` Ohad Ben-Cohen
2010-10-23 14:18 ` Maxim Levitsky [this message]
2010-10-23 14:18 ` Maxim Levitsky
2010-10-23 14:44 ` Ohad Ben-Cohen
2010-10-23 14:44 ` Ohad Ben-Cohen
2010-10-11 8:10 ` 2.6.35.6 fails to suspend (pxa2xx-mci.0) Sven Neumann
2010-10-11 8:10 ` Sven Neumann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1287843531.29752.10.camel@maxim-laptop \
--to=maximlevitsky@gmail.com \
--cc=ccross@android.com \
--cc=cjb@laptop.org \
--cc=daniel@caiaq.de \
--cc=gregkh@suse.de \
--cc=libertas-dev@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=nico@fluxnic.net \
--cc=ohad@wizery.com \
--cc=rjw@sisk.pl \
--cc=s.neumann@raumfeld.com \
--cc=stable@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.