Re: [PATCH V13 03/10] mmc: block: Add blk-mq support

[Ulf Hansson]

On 15 November 2017 at 14:07, Adrian Hunter  wrote:
> On 15/11/17 12:55, Ulf Hansson wrote:
>> Linus, Adrian,
>>
>> Apologize for sidetracking the discussion, just wanted to add some
>> minor comments.
>>
>> [...]
>>
>>>
> But what I think is nice in doing it around
> each request is that since mmc_put_card() calls mmc_release_host()
> contains this:
>
> if (--host->claim_cnt) { (...)
>
> So there is a counter inside the claim/release host functions
> and now there is another counter keeping track if the in-flight
> requests as well and it gives me the feeling we are maintaining
> two counters when we only need one.

 The gets / puts also get runtime pm for the card.  It is a lot a messing
 around for the sake of a quick check for the number of requests inflight -
 which is needed anyway for CQE.
>>
>> Actually the get / puts for runtime PM of the card is already done
>> taking the host->claim_cnt into account.
>
> We do pm_runtime_get_sync(>dev) always i.e.
>
> void mmc_get_card(struct mmc_card *card, struct mmc_ctx *ctx)
> {
> pm_runtime_get_sync(>dev);
> __mmc_claim_host(card->host, ctx, NULL);
> }

You are absolutely correct, so let's leave the inflight counter in.
Apologize for the noise!

I was thinking about the runtime PM of the host dev, which is managed
by mmc_claim|release host().

[...]

 Well I saw 3% drop in sequential read performance, improving to 1% when an
 unbound workqueue was used.
>>
>> Can you please make this improvement as a standalone patch on top of
>> the mq patch?
>
> Unfortunately it was just a hack to the blk-mq core - the block layer does
> not have an unbound workqueue.  I have not had time to consider a proper
> fix.  It will have to wait, but I agree 3% is not enough to delay going 
> forward.

I see, thanks!

[...]

Kind regards
Uffe

Re: [PATCH V13 03/10] mmc: block: Add blk-mq support

[Adrian Hunter]

On 15/11/17 12:55, Ulf Hansson wrote:
> Linus, Adrian,
> 
> Apologize for sidetracking the discussion, just wanted to add some
> minor comments.
> 
> [...]
> 
>>
 But what I think is nice in doing it around
 each request is that since mmc_put_card() calls mmc_release_host()
 contains this:

 if (--host->claim_cnt) { (...)

 So there is a counter inside the claim/release host functions
 and now there is another counter keeping track if the in-flight
 requests as well and it gives me the feeling we are maintaining
 two counters when we only need one.
>>>
>>> The gets / puts also get runtime pm for the card.  It is a lot a messing
>>> around for the sake of a quick check for the number of requests inflight -
>>> which is needed anyway for CQE.
> 
> Actually the get / puts for runtime PM of the card is already done
> taking the host->claim_cnt into account.

We do pm_runtime_get_sync(>dev) always i.e.

void mmc_get_card(struct mmc_card *card, struct mmc_ctx *ctx)
{
pm_runtime_get_sync(>dev);
__mmc_claim_host(card->host, ctx, NULL);
}

> 
> In other words, the additional in-flight counter does not provide an
> additional improvement in this regards.
> 
> For that reason, perhaps the in-flight counter should be added in the
> CQE patch instead, because it seems like it is there it really
> belongs?
> 
> [...]
> 
>>
>> OK I guess I'm more forging ahead with such things. But we could
>> at least enable it by default so whoever checks out and builds
>> and tests linux-next with their MMC/SD controllers will then be
>> testing this for us in the next kernel cycle.
>>
 But I think I would prefer that if a big slew of new code is
 introduced it needs to be put to wide use and any bugs
 smoked out during the -rc phase, and we are now
 hiding it behind a Kconfig option so it's unlikely to see testing
 until that option is turned on, and that is not good.
>>>
>>> And if you find a big problem in rc7, then what do you do?  At least with
>>> the config option, the revert is trivial.
>>
>> I see your point. I guess it is up to Ulf how he feels about
>> trust wrt the code. If a problem appears at -rc7 it's indeed
>> nice if we can leave the code in-tree and work on it.
> 
> Well, it's not an easy decision, simply because it moves the code in
> an even worse situation maintenance wise. So, if you guys just
> suddenly have to move on to do something else, then it becomes my
> problem to work out. :-)
> 
> However, as I trust both of you, and that you seems to agree on the
> path forward, I am fine keeping the old code for while.
> 
> Although, please make sure the Kconfig option starts out by being
> enabled by default, so we can get some test coverage of the mq path.

Ok

> 
> Of course, then we need to work on the card removal problem asap, and
> hopefully we manage to fix them. If not, or other strange errors
> happens, we would need to change the default value to 'n' of the
> Kconfig.
> 
> [...]
> 
 Hm! What you are saying sounds correct, and we really need to
 consider the multi-CPU aspects of this, maybe not now. I am
 happy as long as we have equal performance as before and
 maintainable code.
>>>
>>> Well I saw 3% drop in sequential read performance, improving to 1% when an
>>> unbound workqueue was used.
> 
> Can you please make this improvement as a standalone patch on top of
> the mq patch?

Unfortunately it was just a hack to the blk-mq core - the block layer does
not have an unbound workqueue.  I have not had time to consider a proper
fix.  It will have to wait, but I agree 3% is not enough to delay going forward.

> 
> I think that would be good, because it points out some generic
> problems with mq, which we then, at least short term, tries to address
> locally in MMC.
> 
> [...]
> 
>>
 If you just make a series also deleteing the old block layer
 I will test it so it doesn't break anything and then you can
 probably expect a big Acked-by on the whole shebang.
>>>
>>> I will add patches for that - let's see what happens.
> 
> Yes, please. However, I will as stated, be withholding that change for
> a while, until we fixed any showstoppers in the mq path.

Re: [PATCH V13 03/10] mmc: block: Add blk-mq support

[Ulf Hansson]

Linus, Adrian,

Apologize for sidetracking the discussion, just wanted to add some
minor comments.

[...]

>
>>> But what I think is nice in doing it around
>>> each request is that since mmc_put_card() calls mmc_release_host()
>>> contains this:
>>>
>>> if (--host->claim_cnt) { (...)
>>>
>>> So there is a counter inside the claim/release host functions
>>> and now there is another counter keeping track if the in-flight
>>> requests as well and it gives me the feeling we are maintaining
>>> two counters when we only need one.
>>
>> The gets / puts also get runtime pm for the card.  It is a lot a messing
>> around for the sake of a quick check for the number of requests inflight -
>> which is needed anyway for CQE.

Actually the get / puts for runtime PM of the card is already done
taking the host->claim_cnt into account.

In other words, the additional in-flight counter does not provide an
additional improvement in this regards.

For that reason, perhaps the in-flight counter should be added in the
CQE patch instead, because it seems like it is there it really
belongs?

[...]

>
> OK I guess I'm more forging ahead with such things. But we could
> at least enable it by default so whoever checks out and builds
> and tests linux-next with their MMC/SD controllers will then be
> testing this for us in the next kernel cycle.
>
>>> But I think I would prefer that if a big slew of new code is
>>> introduced it needs to be put to wide use and any bugs
>>> smoked out during the -rc phase, and we are now
>>> hiding it behind a Kconfig option so it's unlikely to see testing
>>> until that option is turned on, and that is not good.
>>
>> And if you find a big problem in rc7, then what do you do?  At least with
>> the config option, the revert is trivial.
>
> I see your point. I guess it is up to Ulf how he feels about
> trust wrt the code. If a problem appears at -rc7 it's indeed
> nice if we can leave the code in-tree and work on it.

Well, it's not an easy decision, simply because it moves the code in
an even worse situation maintenance wise. So, if you guys just
suddenly have to move on to do something else, then it becomes my
problem to work out. :-)

However, as I trust both of you, and that you seems to agree on the
path forward, I am fine keeping the old code for while.

Although, please make sure the Kconfig option starts out by being
enabled by default, so we can get some test coverage of the mq path.

Of course, then we need to work on the card removal problem asap, and
hopefully we manage to fix them. If not, or other strange errors
happens, we would need to change the default value to 'n' of the
Kconfig.

[...]

>>> Hm! What you are saying sounds correct, and we really need to
>>> consider the multi-CPU aspects of this, maybe not now. I am
>>> happy as long as we have equal performance as before and
>>> maintainable code.
>>
>> Well I saw 3% drop in sequential read performance, improving to 1% when an
>> unbound workqueue was used.

Can you please make this improvement as a standalone patch on top of
the mq patch?

I think that would be good, because it points out some generic
problems with mq, which we then, at least short term, tries to address
locally in MMC.

[...]

>
>>> If you just make a series also deleteing the old block layer
>>> I will test it so it doesn't break anything and then you can
>>> probably expect a big Acked-by on the whole shebang.
>>
>> I will add patches for that - let's see what happens.

Yes, please. However, I will as stated, be withholding that change for
a while, until we fixed any showstoppers in the mq path.

[...]

Kind regards
Uffe

Re: [PATCH V13 03/10] mmc: block: Add blk-mq support

[Linus Walleij]

Hi Adrian!

Thanks for the helpful and productive tone in the recent mail,
it is very helpful and improves my work environment.

>> The block layer maintainers most definately think MQ is ready
>> but you seem to disagree. Why?
>
> I meant when the mmc conversion to blk-mq is ready.  We don't need to
> consider other sub-systems, just whether it is working for mmc.  For
> example, the issue with the block layer workqueues not performing as well as
> a thread.  That only came to light through testing mmc.

OK I see your point.

>> It's bad enough that you repeatedly point out how stupid
>> you think I am
>
> I have never called you names.  On the contrary, it is because I don't think
> you are stupid that I get upset when you seem spend so little time and
> effort to understand the code.  Equally, you seem to make comments that just
> assume the code is wrong without due consideration.

Please work on your patience. I think so few people are reviewing the
code because it is massive and complicated and sits in a complicated
place. One can not be blamed for trying, right?

I have spent considerable time with your code, more than with my own.
Mostly testing it, and that is why I can say it does not regress performance
on my end.

I also ran several rounds of fault injection, which it handled without
problems.

Then I tried to eject the SD card when running DD from the card and
then it crashed.

But that is an extreme test case, so overall it is very robust code.
With the exception of card removal during I/O, feel free to add
my Tested-by: Linus Walleij  on this series.

>> But what I think is nice in doing it around
>> each request is that since mmc_put_card() calls mmc_release_host()
>> contains this:
>>
>> if (--host->claim_cnt) { (...)
>>
>> So there is a counter inside the claim/release host functions
>> and now there is another counter keeping track if the in-flight
>> requests as well and it gives me the feeling we are maintaining
>> two counters when we only need one.
>
> The gets / puts also get runtime pm for the card.  It is a lot a messing
> around for the sake of a quick check for the number of requests inflight -
> which is needed anyway for CQE.

OK I buy that.

>> But maybe it is actually the host->claim_cnt that is overengineered
>> and should just be a bool, becuase with this construction
>> that you only call down and claim the host once, the
>> host->claim_cnt will only be 0 or 1, right?
>
> The claim_cnt was introduced for nested claiming.

Yeah that sounds familiar. I'm not really sure it happens
anymore after this but I guess I can test that with some
practical experiements, let's leave it like this.

>> I guess I could turn that around and ask: if it is so crappy, why
>> is your patch set not deleting it?
>
> To allow people time to test.

OK I guess I'm more forging ahead with such things. But we could
at least enable it by default so whoever checks out and builds
and tests linux-next with their MMC/SD controllers will then be
testing this for us in the next kernel cycle.

>> But I think I would prefer that if a big slew of new code is
>> introduced it needs to be put to wide use and any bugs
>> smoked out during the -rc phase, and we are now
>> hiding it behind a Kconfig option so it's unlikely to see testing
>> until that option is turned on, and that is not good.
>
> And if you find a big problem in rc7, then what do you do?  At least with
> the config option, the revert is trivial.

I see your point. I guess it is up to Ulf how he feels about
trust wrt the code. If a problem appears at -rc7 it's indeed
nice if we can leave the code in-tree and work on it.

>> At least take a moment to consider the option that maybe so few people
>> are reviwing your code because this is complicated stuff and really
>> hard to grasp, maybe the problem isn't on my side, neither on yours,
>> it may be that the subject matter is the problem.
>
> I would expect a review would take a number of days, perhaps longer.
> However, it seems people are not willing to consider anything that takes
> more than a an hour or two.

Your criticism is something like "do it properly or not at all"
and I understand that feeling. I deserve some of the criticism
and I can accept it much easier when put with these words.

It would be nice if the community would step in here and I have
seen so many people talk about this code that I really think they
should invest in proper review. More people than me and Ulf
need to help out with review and test.

>> I would say a synchronization primitive is needed, indeed.
>> I have a different approach to this, which uses completion
>> as the synchronization primitive and I think that would be possible
>> to use also here.
>
> Because they are both just wake-ups.  When waiting for multiple conditions,
> wait_event() is simpler.  You are not considering the possibility for having
> only 1 task switch between requests.

That is indeed a nice feature.

>>
>> I have 

Re: [PATCH V13 03/10] mmc: block: Add blk-mq support

[Adrian Hunter]

On 09/11/17 17:52, Linus Walleij wrote:
> On Thu, Nov 9, 2017 at 11:42 AM, Adrian Hunter  
> wrote:
>> On 08/11/17 10:54, Linus Walleij wrote:
>>> On Fri, Nov 3, 2017 at 2:20 PM, Adrian Hunter  
>>> wrote:
> 
>>> At least you could do what I did and break out a helper like
>>> this:
>>>
>>> /*
>>>  * This reports status back to the block layer for a finished request.
>>>  */
>>> static void mmc_blk_complete(struct mmc_queue_req *mq_rq,
>>> blk_status_t status)
>>> {
>>>struct request *req = mmc_queue_req_to_req(mq_rq);
>>>
>>>if (req->mq_ctx) {
>>>   blk_mq_end_request(req, status);
>>>} else
>>>   blk_end_request_all(req, status);
>>> }
>>
>> You are quibbling.
> 
> Hm wiktionary "quibble" points to:
> https://en.wikipedia.org/wiki/Trivial_objections
> 
> Sorry for not being a native speaker in English, I think that
> maybe you were again trying to be snarky to a reviewer but
> I honestly don't know.
> 
>> It makes next to no difference especially as it all goes
>> away when the legacy code is deleted.
> 
> Which is something your patch set doesn't do. Nor did you write
> anywhere that deleting the legacy code was your intention.
> But I'm happy to hear it.
> 
>>  I will change it in the next
>> revision, but what a waste of everyone's time.
>>  Please try to focus on
>> things that matter.
> 
> As Jean-Paul Sartre said "hell is other people".
> Can you try to be friendlier anyway?
> 
>>> This retry and error handling using requeue is very elegant.
>>> I really like this.
>>>
>>> If we could also go for MQ-only, only this nice code
>>> remains in the tree.
>>
>> No one has ever suggested that the legacy API will remain.  Once blk-mq is
>> ready the old code gets deleted.
> 
> The block layer maintainers most definately think MQ is ready
> but you seem to disagree. Why?

I meant when the mmc conversion to blk-mq is ready.  We don't need to
consider other sub-systems, just whether it is working for mmc.  For
example, the issue with the block layer workqueues not performing as well as
a thread.  That only came to light through testing mmc.

> 
> In the recent LWN article from kernel recepies:
> https://lwn.net/Articles/735275/
> the only obstacle I can see is a mention that SCSI was not
> switched over by default because of some problems with
> slow rotating media. "This change had to be backed out
> because of some performance and scalability issues that
> arose for rotating storage."
> 
> Christoph mentions that he's gonna try again for v4.14.
> But it's true, I do not see that happening in linux-next yet.
> 
> But that is specifically rotating media, which MMC/SD is not.
> And UBI is using it since ages without complaints. And
> that is a sign of something.
> 
> Have you seen any problems when deploying it on MMC/SD,
> really? If there are problems I bet the block layer people want
> us to find them, diagnose them and ultimately fix them rather
> than wait for them to do it. I haven't seen any performance or
> scalability issues. At one point I had processes running
> on two eMMC and one SD-card and apart from maxing out
> the CPUs and DMA backplace as could be expected all was
> fine.
> 
>>> The problem: you have just reimplemented the whole error
>>> handling we had in the old block layer and now we have to
>>> maintain two copies and keep them in sync.
>>>
>>> This is not OK IMO, we will inevitable screw it up, so we
>>> need to get *one* error path.
>>
>> Wow, you really didn't read the code at all.
> 
> Just quit this snarky attitude.
> 
>> As I have repeatedly pointed
>> out, the new code is all new.  There is no overlap and there nothing to keep
>> in sync.
> 
> The problem is that you have not clearly communicated your
> vision to delete the old code. The best way to communicate that
> would be to include a patch actually deleteing it.
> 
>>  It may not look like it in this patch, but that is only because of
>> the ridiculous idea of splitting up the patch.
> 
> Naming other people's review feedback as "ridiculous" is not very
> helpful. I think the patch set became much easier to review
> like this so I am happy with this format.
> 
 +static void mmc_blk_mq_acct_req_done(struct mmc_queue *mq, struct request 
 *req)
>>>
>>> What does "acct" mean in the above function name?
>>> Accounting? Actual? I'm lost.
>>
>> Does "actual" have two "c"'s.  You are just making things up.
> 
> Please stop your snarky and belitteling attitude.
> 
> I am not a native English speaker and I am not making up
> my review comments in order to annoy you.
> 
> Consider Hanlon's razor:
> https://en.wikipedia.org/wiki/Hanlon%27s_razor
> "Never attribute to malice that which is adequately explained by
> stupidity".
> 
> It's bad enough that you repeatedly point out how stupid
> you think I am

I have never called you names.  On the contrary, it is because I don't think
you are stupid that I get upset 

Re: [PATCH V13 03/10] mmc: block: Add blk-mq support

[Linus Walleij]

Following up on this:

On Thu, Nov 9, 2017 at 4:52 PM, Linus Walleij  wrote:

>> No one has ever suggested that the legacy API will remain.  Once blk-mq is
>> ready the old code gets deleted.
>
> The block layer maintainers most definately think MQ is ready
> but you seem to disagree. Why?
>
> In the recent LWN article from kernel recepies:
> https://lwn.net/Articles/735275/
> the only obstacle I can see is a mention that SCSI was not
> switched over by default because of some problems with
> slow rotating media. "This change had to be backed out
> because of some performance and scalability issues that
> arose for rotating storage."
>
> Christoph mentions that he's gonna try again for v4.14.
> But it's true, I do not see that happening in linux-next yet.

Neil Brown's article on LWN points to Ming Lei's patch
set:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1513023.html

As addressing the issue that held back blk-MQ from
being default for SCSI, and it is reportedly to be
queued for v4.15. Since this MMC/SD rework is
also targeted for v4.15 I think we can assume it is
pretty much ready for everything, and delete the
non-MQ block path.

I just haven't seen any of this problem in my tests
with MMC/SD so I do not think it would be affected,
but it anyways seem to be fixed.

OK maybe I am especially optimistic. But it's not just
based on that.

Yours,
Linus Walleij

Re: [PATCH V13 03/10] mmc: block: Add blk-mq support

[Linus Walleij]

On Thu, Nov 9, 2017 at 11:42 AM, Adrian Hunter  wrote:
> On 08/11/17 10:54, Linus Walleij wrote:
>> On Fri, Nov 3, 2017 at 2:20 PM, Adrian Hunter  
>> wrote:

>> At least you could do what I did and break out a helper like
>> this:
>>
>> /*
>>  * This reports status back to the block layer for a finished request.
>>  */
>> static void mmc_blk_complete(struct mmc_queue_req *mq_rq,
>> blk_status_t status)
>> {
>>struct request *req = mmc_queue_req_to_req(mq_rq);
>>
>>if (req->mq_ctx) {
>>   blk_mq_end_request(req, status);
>>} else
>>   blk_end_request_all(req, status);
>> }
>
> You are quibbling.

Hm wiktionary "quibble" points to:
https://en.wikipedia.org/wiki/Trivial_objections

Sorry for not being a native speaker in English, I think that
maybe you were again trying to be snarky to a reviewer but
I honestly don't know.

> It makes next to no difference especially as it all goes
> away when the legacy code is deleted.

Which is something your patch set doesn't do. Nor did you write
anywhere that deleting the legacy code was your intention.
But I'm happy to hear it.

>  I will change it in the next
> revision, but what a waste of everyone's time.
>  Please try to focus on
> things that matter.

As Jean-Paul Sartre said "hell is other people".
Can you try to be friendlier anyway?

>> This retry and error handling using requeue is very elegant.
>> I really like this.
>>
>> If we could also go for MQ-only, only this nice code
>> remains in the tree.
>
> No one has ever suggested that the legacy API will remain.  Once blk-mq is
> ready the old code gets deleted.

The block layer maintainers most definately think MQ is ready
but you seem to disagree. Why?

In the recent LWN article from kernel recepies:
https://lwn.net/Articles/735275/
the only obstacle I can see is a mention that SCSI was not
switched over by default because of some problems with
slow rotating media. "This change had to be backed out
because of some performance and scalability issues that
arose for rotating storage."

Christoph mentions that he's gonna try again for v4.14.
But it's true, I do not see that happening in linux-next yet.

But that is specifically rotating media, which MMC/SD is not.
And UBI is using it since ages without complaints. And
that is a sign of something.

Have you seen any problems when deploying it on MMC/SD,
really? If there are problems I bet the block layer people want
us to find them, diagnose them and ultimately fix them rather
than wait for them to do it. I haven't seen any performance or
scalability issues. At one point I had processes running
on two eMMC and one SD-card and apart from maxing out
the CPUs and DMA backplace as could be expected all was
fine.

>> The problem: you have just reimplemented the whole error
>> handling we had in the old block layer and now we have to
>> maintain two copies and keep them in sync.
>>
>> This is not OK IMO, we will inevitable screw it up, so we
>> need to get *one* error path.
>
> Wow, you really didn't read the code at all.

Just quit this snarky attitude.

> As I have repeatedly pointed
> out, the new code is all new.  There is no overlap and there nothing to keep
> in sync.

The problem is that you have not clearly communicated your
vision to delete the old code. The best way to communicate that
would be to include a patch actually deleteing it.

>  It may not look like it in this patch, but that is only because of
> the ridiculous idea of splitting up the patch.

Naming other people's review feedback as "ridiculous" is not very
helpful. I think the patch set became much easier to review
like this so I am happy with this format.

>>> +static void mmc_blk_mq_acct_req_done(struct mmc_queue *mq, struct request 
>>> *req)
>>
>> What does "acct" mean in the above function name?
>> Accounting? Actual? I'm lost.
>
> Does "actual" have two "c"'s.  You are just making things up.

Please stop your snarky and belitteling attitude.

I am not a native English speaker and I am not making up
my review comments in order to annoy you.

Consider Hanlon's razor:
https://en.wikipedia.org/wiki/Hanlon%27s_razor
"Never attribute to malice that which is adequately explained by
stupidity".

It's bad enough that you repeatedly point out how stupid
you think I am, I am sorry about that, in my opinion sometimes
other people are really stupid but more often than not the problem
is really on your side, like being impatient and unwilling to educate
others about how your code actually works becuase it seems
so evident to yourself that you think it should be evident to everyone
else as well. I don't know what to say about that, it seems a bit
unempatic.

But you even go and think I am making stuff up in purpose.
That's pretty offensive.

> Of course it is "account".

This is maybe obvious for you but it was not to me.

>  It is counting the number of requests in flight - which is
> 

Re: [PATCH V13 03/10] mmc: block: Add blk-mq support

[Linus Walleij]

On Fri, Nov 3, 2017 at 2:20 PM, Adrian Hunter  wrote:

> Define and use a blk-mq queue. Discards and flushes are processed
> synchronously, but reads and writes asynchronously. In order to support
> slow DMA unmapping, DMA unmapping is not done until after the next request
> is started. That means the request is not completed until then. If there is
> no next request then the completion is done by queued work.
>
> Signed-off-by: Adrian Hunter 

> -   blk_end_request_all(req, ret ? BLK_STS_IOERR : BLK_STS_OK);
> +   if (req->mq_ctx)
> +   blk_mq_end_request(req, ret ? BLK_STS_IOERR : BLK_STS_OK);
> +   else
> +   blk_end_request_all(req, ret ? BLK_STS_IOERR : BLK_STS_OK);

I think this quite obvious code duplication is unfortunate.

What my patches do is get rid of the old block layer in order
to be able to focus on the new stuff using just MQ.

One reason is that the code is hairy already as it is, by just
supporting MQ the above is still just one line of code, the same
goes for the other instances below.

At least you could do what I did and break out a helper like
this:

/*
 * This reports status back to the block layer for a finished request.
 */
static void mmc_blk_complete(struct mmc_queue_req *mq_rq,
blk_status_t status)
{
   struct request *req = mmc_queue_req_to_req(mq_rq);

   if (req->mq_ctx) {
  blk_mq_end_request(req, status);
   } else
  blk_end_request_all(req, status);
}

> +/* Single sector read during recovery */
> +static void mmc_blk_ss_read(struct mmc_queue *mq, struct request *req)
> +{
> +   struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> +   blk_status_t status;
> +
> +   while (1) {
> +   mmc_blk_rw_rq_prep(mqrq, mq->card, 1, mq);
> +
> +   mmc_wait_for_req(mq->card->host, >brq.mrq);
> +
> +   /*
> +* Not expecting command errors, so just give up in that case.
> +* If there are retries remaining, the request will get
> +* requeued.
> +*/
> +   if (mqrq->brq.cmd.error)
> +   return;
> +
> +   if (blk_rq_bytes(req) <= 512)
> +   break;
> +
> +   status = mqrq->brq.data.error ? BLK_STS_IOERR : BLK_STS_OK;
> +
> +   blk_update_request(req, status, 512);
> +   }
> +
> +   mqrq->retries = MMC_NO_RETRIES;
> +}
> +
> +static void mmc_blk_rw_recovery(struct mmc_queue *mq, struct request *req)
> +{
> +   int type = rq_data_dir(req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
> +   struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> +   struct mmc_blk_request *brq = >brq;
> +   struct mmc_blk_data *md = mq->blkdata;
> +   struct mmc_card *card = mq->card;
> +   static enum mmc_blk_status status;
> +
> +   brq->retune_retry_done = mqrq->retries;
> +
> +   status = __mmc_blk_err_check(card, mqrq);
> +
> +   mmc_retune_release(card->host);
> +
> +   /*
> +* Requests are completed by mmc_blk_mq_complete_rq() which sets 
> simple
> +* policy:
> +* 1. A request that has transferred at least some data is considered
> +* successful and will be requeued if there is remaining data to
> +* transfer.
> +* 2. Otherwise the number of retries is incremented and the request
> +* will be requeued if there are remaining retries.
> +* 3. Otherwise the request will be errored out.
> +* That means mmc_blk_mq_complete_rq() is controlled by bytes_xfered 
> and
> +* mqrq->retries. So there are only 4 possible actions here:
> +*  1. do not accept the bytes_xfered value i.e. set it to zero
> +*  2. change mqrq->retries to determine the number of retries
> +*  3. try to reset the card
> +*  4. read one sector at a time
> +*/
> +   switch (status) {
> +   case MMC_BLK_SUCCESS:
> +   case MMC_BLK_PARTIAL:
> +   /* Reset success, and accept bytes_xfered */
> +   mmc_blk_reset_success(md, type);
> +   break;
> +   case MMC_BLK_CMD_ERR:
> +   /*
> +* For SD cards, get bytes written, but do not accept
> +* bytes_xfered if that fails. For MMC cards accept
> +* bytes_xfered. Then try to reset. If reset fails then
> +* error out the remaining request, otherwise retry
> +* once (N.B mmc_blk_reset() will not succeed twice in a
> +* row).
> +*/
> +   if (mmc_card_sd(card)) {
> +   u32 blocks;
> +   int err;
> +
> +   err = mmc_sd_num_wr_blocks(card, );
> +   if (err)
> +   brq->data.bytes_xfered = 0;
> +