[1/6] mmc: block: break out mmc_blk_rw_cmd_abort()

Message ID 20170124101757.19676-2-linus.walleij@linaro.org
State New
Headers show
Series
  • mmc: block: command issue cleanups
Related show

Commit Message

Linus Walleij Jan. 24, 2017, 10:17 a.m.
As a first step toward breaking apart the very complex function
mmc_blk_issue_rw_rq() we break out the command abort code.
This code assumes "ret" is != 0 and then repeatedly hammers
blk_end_request() until the request to the block layer to end
the request succeeds.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/mmc/core/block.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

-- 
2.9.3

--
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

Comments

Mateusz Nowak Jan. 25, 2017, 9:23 a.m. | #1
Hi Linus,

On 1/24/2017 11:17, Linus Walleij wrote:
> As a first step toward breaking apart the very complex function

> mmc_blk_issue_rw_rq() we break out the command abort code.

> This code assumes "ret" is != 0 and then repeatedly hammers

> blk_end_request() until the request to the block layer to end

> the request succeeds.

>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  drivers/mmc/core/block.c | 17 ++++++++++++-----

>  1 file changed, 12 insertions(+), 5 deletions(-)

>

> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c

> index 7bd03381810d..14efe92a14ef 100644

> --- a/drivers/mmc/core/block.c

> +++ b/drivers/mmc/core/block.c

> @@ -1598,6 +1598,17 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,

>  	return ret;

>  }

>

> +static void mmc_blk_rw_cmd_abort(struct mmc_card *card, struct request *req)

> +{

> +	int ret = 1;

blk_end_request is returning bool, so maybe this variable should have 
matching type since it is only usage in this scope? And maybe it should 
have more meaningful name for this case?

> +

> +	if (mmc_card_removed(card))

> +		req->rq_flags |= RQF_QUIET;

> +	while (ret)

> +		ret = blk_end_request(req, -EIO,

> +				      blk_rq_cur_bytes(req));

> +}

> +

>  static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)

>  {

>  	struct mmc_blk_data *md = mq->blkdata;

> @@ -1737,11 +1748,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)

>  	return 1;

>

>   cmd_abort:

> -	if (mmc_card_removed(card))

> -		req->rq_flags |= RQF_QUIET;

> -	while (ret)

> -		ret = blk_end_request(req, -EIO,

> -				blk_rq_cur_bytes(req));

> +	mmc_blk_rw_cmd_abort(card, req);

>

>   start_new_req:

>  	if (rqc) {

>


Regards,
Mateusz.
--
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
Linus Walleij Jan. 26, 2017, 3:01 p.m. | #2
On Wed, Jan 25, 2017 at 10:23 AM, Mateusz Nowak
<mateusz.nowak@linux.intel.com> wrote:
> On 1/24/2017 11:17, Linus Walleij wrote:

>>

>> As a first step toward breaking apart the very complex function

>> mmc_blk_issue_rw_rq() we break out the command abort code.

>> This code assumes "ret" is != 0 and then repeatedly hammers

>> blk_end_request() until the request to the block layer to end

>> the request succeeds.

>>

>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

>> ---

>>  drivers/mmc/core/block.c | 17 ++++++++++++-----

>>  1 file changed, 12 insertions(+), 5 deletions(-)

>>

>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c

>> index 7bd03381810d..14efe92a14ef 100644

>> --- a/drivers/mmc/core/block.c

>> +++ b/drivers/mmc/core/block.c

>> @@ -1598,6 +1598,17 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md,

>> struct mmc_card *card,

>>         return ret;

>>  }

>>

>> +static void mmc_blk_rw_cmd_abort(struct mmc_card *card, struct request

>> *req)

>> +{

>> +       int ret = 1;

>

> blk_end_request is returning bool, so maybe this variable should have

> matching type since it is only usage in this scope? And maybe it should have

> more meaningful name for this case?


I am just moving/refactoring the code syntax without changing any
semantics. It was an int in the old code so it is an int in the new
code.

It is possible to fix this and other problems as separate patches.

As you can see it also spins here for all eternity if the function
just returns != 0 all the time, that doesn't look good either.

Yours,
Linus Walleij
--
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

Patch

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 7bd03381810d..14efe92a14ef 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1598,6 +1598,17 @@  static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
 	return ret;
 }
 
+static void mmc_blk_rw_cmd_abort(struct mmc_card *card, struct request *req)
+{
+	int ret = 1;
+
+	if (mmc_card_removed(card))
+		req->rq_flags |= RQF_QUIET;
+	while (ret)
+		ret = blk_end_request(req, -EIO,
+				      blk_rq_cur_bytes(req));
+}
+
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 {
 	struct mmc_blk_data *md = mq->blkdata;
@@ -1737,11 +1748,7 @@  static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 	return 1;
 
  cmd_abort:
-	if (mmc_card_removed(card))
-		req->rq_flags |= RQF_QUIET;
-	while (ret)
-		ret = blk_end_request(req, -EIO,
-				blk_rq_cur_bytes(req));
+	mmc_blk_rw_cmd_abort(card, req);
 
  start_new_req:
 	if (rqc) {