Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210216224252.22187-1-marten.lindahl@axis.com>
 <CAPDyKFoASx=U8b1Oqtuo6ikiM=gXfL2x1Gsz=rfAn9zxP0y_iA@mail.gmail.com> <20210301215923.6jfg6mg5ntorttan@axis.com>
In-Reply-To: <20210301215923.6jfg6mg5ntorttan@axis.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Tue, 2 Mar 2021 09:45:02 +0100
Message-ID: <CAPDyKFoaKfuwweaEMf1Pz+ECAPU3P9-gmCJcpq+MADH5gH1c=Q@mail.gmail.com>
Subject: Re: [PATCH] mmc: Try power cycling card if command request times out
To:     Marten Lindahl <martenli@axis.com>
Cc:     =?UTF-8?Q?M=C3=A5rten_Lindahl?= <Marten.Lindahl@axis.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        kernel <kernel@axis.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, 1 Mar 2021 at 22:59, Marten Lindahl <martenli@axis.com> wrote:
>
> Hi Ulf!
>
> Thank you for your comments!
>
> On Mon, Mar 01, 2021 at 09:50:56AM +0100, Ulf Hansson wrote:
> > + Adrian
> >
> > On Tue, 16 Feb 2021 at 23:43, M=C3=A5rten Lindahl <marten.lindahl@axis.=
com> wrote:
> > >
> > > Sometimes SD cards that has been run for a long time enters a state
> > > where it cannot by itself be recovered, but needs a power cycle to be
> > > operational again. Card status analysis has indicated that the card c=
an
> > > end up in a state where all external commands are ignored by the card
> > > since it is halted by data timeouts.
> > >
> > > If the card has been heavily used for a long time it can be weared ou=
t,
> > > and should typically be replaced. But on some tests, it shows that th=
e
> > > card can still be functional after a power cycle, but as it requires =
an
> > > operator to do it, the card can remain in a non-operational state for=
 a
> > > long time until the problem has been observed by the operator.
> > >
> > > This patch adds function to power cycle the card in case it does not
> > > respond to a command, and then resend the command if the power cycle
> > > was successful. This procedure will be tested 1 time before giving up=
,
> > > and resuming host operation as normal.
> >
> > I assume the context above is all about the ioctl interface?
> >
>
> Yes, that's correct. The problem we have seen is triggered by ioctls.
>
> > So, when the card enters this non functional state, have you tried
> > just reading a block through the regular I/O interface. Does it
> > trigger a power cycle of the card - and then makes it functional
> > again?
> >
>
> Yes, we have tried that, and it does trigger a power cycle, making the ca=
rd
> operational again. But as it requires an operator to trigger it, I though=
t
> it might be something that could be automated here. At least once.

Not sure what you mean by operator here? In the end it's a userspace
program running and I assume it can deal with error paths. :-)

In any case, I understand your point.

>
> > >
> > > Signed-off-by: M=C3=A5rten Lindahl <marten.lindahl@axis.com>
> > > ---
> > > Please note: This might not be the way we want to handle these cases,
> > > but at least it lets us start the discussion. In which cases should t=
he
> > > mmc framework deal with error messages like ETIMEDOUT, and in which
> > > cases should it be handled by userspace?
> > > The mmc framework tries to recover a failed block request
> > > (mmc_blk_mq_rw_recovery) which may end up in a HW reset of the card.
> > > Would it be an idea to act in a similar way when an ioctl times out?
> >
> > Maybe, it's a good idea to allow the similar reset for ioctls as we do
> > for regular I/O requests. My concern with this though, is that we
> > might allow user space to trigger a HW resets a bit too easily - and
> > that could damage the card.
> >
> > Did you consider this?
> >
>
> Yes, that is a valid point, and that is why the power cycle is only tried
> once. But the conditon for this reset is a -ETIMEDOUT, and this is the pa=
rt of
> this patch where I am myself not sure of if it is enough to check for. Wo=
uld
> this be an error that you could expect to happen with ioctl requests in o=
ther
> situations also, but not necessarily cause by a stalled card?

Exactly.

Many different commands can get pushed down to the card through the
mmc ioctl interface. It's difficult to know what error path we should
pick, other than reporting and propagating the error codes.

[...]

Kind regards
Uffe