mmc: core: fix the aysync mechanism when card removed

PATCH In review Jaehoon Chung 3 participants 6 messages Jan 22, 2013 – Jan 31, 2013 MMC, SD and SDIO

Share & tools

Jaehoon Chung

author

2013-01-22 12:18 UTC #0 lore raw

PATCH 1/1 mmc: core: fix the aysync mechanism when card removed

When card removed, then didn't complete the previously data. (It didn't wakeup any interrupt.)
If card is removed, then we can assume to complete the previously data. And wakeup the interrupt for wait_event of data.

This problem is produced when sd-card is removed.(Sd-card is running some operation)

Signed-off-by Jaehoon Chung <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/mmc/core/core.c |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

drivers/mmc/core/core.c +20 ~~-11~~

index 8b3a122..bc1d627 100644
@@ -515,17 +515,26 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
 		mmc_pre_req(host, areq->mrq, !host->areq);
 
 	if (host->areq) {
-			err = mmc_wait_for_data_req_done(host, host->areq->mrq,
-					areq);
-			if (err == MMC_BLK_NEW_REQUEST) {
-				if (error)
-					*error = err;
-				/*
-				 * The previous request was not completed,
-				 * nothing to return
-				 */
-				return NULL;
-			}
+		err = mmc_wait_for_data_req_done(host, host->areq->mrq,
+				areq);
+		if (err == MMC_BLK_NEW_REQUEST) {
+			if (error)
+				*error = err;
+			/*
+			 * The previous request was not completed,
+			 * nothing to return
+			 */
+			return NULL;
+		} else if (err == MMC_BLK_NOMEDIUM && areq) {
+			struct mmc_context_info *ctnx = &host->context_info;
+			/*
+			 * If crad is removed,
+			 * then we didn't wait for data completed.
+			 * Assume that data-recieve done.
+			 */
+			ctnx->is_done_rcv = true;
+			wake_up_interruptible(&ctnx->wait);
+		}
 		/*
 		 * Check BKOPS urgency for each R1 response
 		 */

Signature · 1 line

1.7.5.4

Jaehoon Chung

author

2013-01-31 07:06 UTC #1 lore raw

Have any other opinion?

Quoted 56 lines · Jaehoon Chung

This problem is produced when sd-card is removed.(Sd-card is running some operation)

Signed-off-by Jaehoon Chung <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/mmc/core/core.c |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

drivers/mmc/core/core.c +20 ~~-11~~

index 8b3a122..bc1d627 100644
@@ -515,17 +515,26 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
 		mmc_pre_req(host, areq->mrq, !host->areq);
 
 	if (host->areq) {
-			err = mmc_wait_for_data_req_done(host, host->areq->mrq,
-					areq);
-			if (err == MMC_BLK_NEW_REQUEST) {
-				if (error)
-					*error = err;
-				/*
-				 * The previous request was not completed,
-				 * nothing to return
-				 */
-				return NULL;
-			}
+		err = mmc_wait_for_data_req_done(host, host->areq->mrq,
+				areq);
+		if (err == MMC_BLK_NEW_REQUEST) {
+			if (error)
+				*error = err;
+			/*
+			 * The previous request was not completed,
+			 * nothing to return
+			 */
+			return NULL;
+		} else if (err == MMC_BLK_NOMEDIUM && areq) {
+			struct mmc_context_info *ctnx = &host->context_info;
+			/*
+			 * If crad is removed,
+			 * then we didn't wait for data completed.
+			 * Assume that data-recieve done.
+			 */
+			ctnx->is_done_rcv = true;
+			wake_up_interruptible(&ctnx->wait);
+		}
 		/*
 		 * Check BKOPS urgency for each R1 response
 		 */

Subhash Jadavani

2013-01-31 08:42 UTC #2 lore raw

In reply to Jaehoon Chung

Quoted 41 lines · Jaehoon Chung

This problem is produced when sd-card is removed.(Sd-card is running some operation)

Signed-off-by Jaehoon Chung <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
  drivers/mmc/core/core.c |   31 ++++++++++++++++++++-----------
  1 files changed, 20 insertions(+), 11 deletions(-)

drivers/mmc/core/core.c +9 ~~-11~~

index 8b3a122..bc1d627 100644
@@ -515,17 +515,26 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
  		mmc_pre_req(host, areq->mrq, !host->areq);
 
  	if (host->areq) {
-			err = mmc_wait_for_data_req_done(host, host->areq->mrq,
-					areq);
-			if (err == MMC_BLK_NEW_REQUEST) {
-				if (error)
-					*error = err;
-				/*
-				 * The previous request was not completed,
-				 * nothing to return
-				 */
-				return NULL;
-			}
+		err = mmc_wait_for_data_req_done(host, host->areq->mrq,
+				areq);
+		if (err == MMC_BLK_NEW_REQUEST) {
+			if (error)
+				*error = err;
+			/*
+			 * The previous request was not completed,
+			 * nothing to return
+			 */

drivers/mmc/core/core.c struct mmc_async_req *mmc_start_req(struct mmc_host *host, · line 536

Quoted 10 lines

drivers/mmc/core/core.c +10 -0

+			return NULL;
+		} else if (err == MMC_BLK_NOMEDIUM && areq) {
+			struct mmc_context_info *ctnx = &host->context_info;
+			/*
+			 * If crad is removed,
+			 * then we didn't wait for data completed.
+			 * Assume that data-recieve done.
+			 */
+			ctnx->is_done_rcv = true;
+			wake_up_interruptible(&ctnx->wait);

Can you please explain more on what exactly we are trying to do here (may be you can list down the call flow)? I am not sure if i understood this correctly. But why would you need to wakeup the thread here? We are here because thread is already woken up. mmc_wait_for_data_req_done() would return only if either the new request is received or the currently running request on controller is completed.

Regards,
Subhash

Quoted 4 lines

drivers/mmc/core/core.c +1 -0

+		}
  		/*
  		 * Check BKOPS urgency for each R1 response
  		 */

Jaehoon Chung

author

2013-01-31 09:46 UTC #3 lore raw

Quoted 45 lines · Subhash Jadavani

Quoted 44 lines · Jaehoon Chung

This problem is produced when sd-card is removed.(Sd-card is running some operation)

Signed-off-by Jaehoon Chung <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
  drivers/mmc/core/core.c |   31 ++++++++++++++++++++-----------
  1 files changed, 20 insertions(+), 11 deletions(-)

drivers/mmc/core/core.c +13 ~~-11~~

index 8b3a122..bc1d627 100644
@@ -515,17 +515,26 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
          mmc_pre_req(host, areq->mrq, !host->areq);
        if (host->areq) {
-            err = mmc_wait_for_data_req_done(host, host->areq->mrq,
-                    areq);
-            if (err == MMC_BLK_NEW_REQUEST) {
-                if (error)
-                    *error = err;
-                /*
-                 * The previous request was not completed,
-                 * nothing to return
-                 */
-                return NULL;
-            }
+        err = mmc_wait_for_data_req_done(host, host->areq->mrq,
+                areq);
+        if (err == MMC_BLK_NEW_REQUEST) {
+            if (error)
+                *error = err;
+            /*
+             * The previous request was not completed,
+             * nothing to return
+             */
+            return NULL;
+        } else if (err == MMC_BLK_NOMEDIUM && areq) {
+            struct mmc_context_info *ctnx = &host->context_info;
+            /*

Quoted 8 lines

drivers/mmc/core/core.c struct mmc_async_req *mmc_start_req(struct mmc_host *host, · line 536
Quoted 6 lines
drivers/mmc/core/core.c +6 -0
+             * If crad is removed,
+             * then we didn't wait for data completed.
+             * Assume that data-recieve done.
+             */
+            ctnx->is_done_rcv = true;
+            wake_up_interruptible(&ctnx->wait);
Can you please explain more on what exactly we are trying to do here (may be you can list down the call flow)? I am not sure if i understood this correctly. But why would you need to wakeup the thread here? We are here because thread is already woken up. mmc_wait_for_data_req_done() would return only if either the new request is received or the currently running request on controller is completed.

This problem is produced when card is removed and running some operation.
If card is removed, mmc_wait_for_data_req_done() is returned error, right?
But if there was async request and running async operation, then maybe run mmc_start_request() with areq. controller is waiting "_for_req_data_done_" for areq.

If request is host->areq.process is running at this time.
But if request is areq(prepared request), then mmc_wait_for_data_req_done should be pending. and system is dead-lock.

Best Regards,
Jaehoon Chung

Quoted 14 lines

Regards,
Subhash

Quoted 4 lines

drivers/mmc/core/core.c +1 -0

+        }
          /*
           * Check BKOPS urgency for each R1 response
           */

Signature · 3 lines

To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Seungwon Jeon

2013-01-31 10:48 UTC #4 lore raw

Quoted 63 lines · Jaehoon Chung

Quoted 45 lines · Subhash Jadavani
Quoted 44 lines · Jaehoon Chung
When card removed, then didn't complete the previously data. (It didn't wakeup any interrupt.)
If card is removed, then we can assume to complete the previously data. And wakeup the interrupt for wait_event of data.
This problem is produced when sd-card is removed.(Sd-card is running some operation)
Signed-off-by Jaehoon Chung <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
  drivers/mmc/core/core.c |   31 ++++++++++++++++++++-----------
  1 files changed, 20 insertions(+), 11 deletions(-)
drivers/mmc/core/core.c +13 ~~-11~~
index 8b3a122..bc1d627 100644
@@ -515,17 +515,26 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
          mmc_pre_req(host, areq->mrq, !host->areq);
        if (host->areq) {
-            err = mmc_wait_for_data_req_done(host, host->areq->mrq,
-                    areq);
-            if (err == MMC_BLK_NEW_REQUEST) {
-                if (error)
-                    *error = err;
-                /*
-                 * The previous request was not completed,
-                 * nothing to return
-                 */
-                return NULL;
-            }
+        err = mmc_wait_for_data_req_done(host, host->areq->mrq,
+                areq);
+        if (err == MMC_BLK_NEW_REQUEST) {
+            if (error)
+                *error = err;
+            /*
+             * The previous request was not completed,
+             * nothing to return
+             */
+            return NULL;
+        } else if (err == MMC_BLK_NOMEDIUM && areq) {
+            struct mmc_context_info *ctnx = &host->context_info;
+            /*
Quoted 8 lines
drivers/mmc/core/core.c struct mmc_async_req *mmc_start_req(struct mmc_host *host, · line 536
Quoted 6 lines
drivers/mmc/core/core.c +6 -0
+             * If crad is removed,
+             * then we didn't wait for data completed.
+             * Assume that data-recieve done.
+             */
+            ctnx->is_done_rcv = true;
+            wake_up_interruptible(&ctnx->wait);
Can you please explain more on what exactly we are trying to do here (may be you can list down the
call flow)? I am not sure if i understood this correctly. But why would you need to wakeup the thread here? We are here because thread is already woken up. mmc_wait_for_data_req_done() would return only if either the new request is received or the currently running request on controller is completed. This problem is produced when card is removed and running some operation.
If card is removed, mmc_wait_for_data_req_done() is returned error, right?
But if there was async request and running async operation, then maybe run mmc_start_request() with areq.
controller is waiting "_for_req_data_done_" for areq.

Quoted 3 lines

If request is host->areq.process is running at this time.
But if request is areq(prepared request), then mmc_wait_for_data_req_done should be pending. and system is dead-lock.

I also found this problem with sdcard.
I think it can be solved easily. Did you see the following patch? [PATCH 1/2] mmc: core: fix permanent sleep of mmcqd during card removal We may just get the hint from __mmc_start_req function.

Thanks,
Seungwon Jeon

Quoted 23 lines

Best Regards,
Jaehoon Chung
Quoted 9 lines
Regards,
Subhash
drivers/mmc/core/core.c struct mmc_async_req *mmc_start_req(struct mmc_host *host, · line 540
Quoted 4 lines
drivers/mmc/core/core.c +1 -0
+        }
          /*
           * Check BKOPS urgency for each R1 response
           */
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Jaehoon Chung

author

2013-01-31 11:04 UTC #5 lore raw

Hi Seungwon,

I didn't see your patch "[PATCH 1/2] mmc: core: fix permanent sleep of mmcqd during card removal". But i found the patch. right..i think it's also same problem.

Your patch is more generic..great.
I will reply to your patch with my acked.

Best Regards,
Jaehoon Chung

Quoted 103 lines · Seungwon Jeon

Quoted 63 lines · Jaehoon Chung
Quoted 45 lines · Subhash Jadavani
Quoted 44 lines · Jaehoon Chung
When card removed, then didn't complete the previously data. (It didn't wakeup any interrupt.)
If card is removed, then we can assume to complete the previously data. And wakeup the interrupt for wait_event of data.
This problem is produced when sd-card is removed.(Sd-card is running some operation)
Signed-off-by Jaehoon Chung <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
  drivers/mmc/core/core.c |   31 ++++++++++++++++++++-----------
  1 files changed, 20 insertions(+), 11 deletions(-)
drivers/mmc/core/core.c +13 ~~-11~~
index 8b3a122..bc1d627 100644
@@ -515,17 +515,26 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
          mmc_pre_req(host, areq->mrq, !host->areq);
        if (host->areq) {
-            err = mmc_wait_for_data_req_done(host, host->areq->mrq,
-                    areq);
-            if (err == MMC_BLK_NEW_REQUEST) {
-                if (error)
-                    *error = err;
-                /*
-                 * The previous request was not completed,
-                 * nothing to return
-                 */
-                return NULL;
-            }
+        err = mmc_wait_for_data_req_done(host, host->areq->mrq,
+                areq);
+        if (err == MMC_BLK_NEW_REQUEST) {
+            if (error)
+                *error = err;
+            /*
+             * The previous request was not completed,
+             * nothing to return
+             */
+            return NULL;
+        } else if (err == MMC_BLK_NOMEDIUM && areq) {
+            struct mmc_context_info *ctnx = &host->context_info;
+            /*
Quoted 8 lines
drivers/mmc/core/core.c struct mmc_async_req *mmc_start_req(struct mmc_host *host, · line 536
Quoted 6 lines
drivers/mmc/core/core.c +6 -0
+             * If crad is removed,
+             * then we didn't wait for data completed.
+             * Assume that data-recieve done.
+             */
+            ctnx->is_done_rcv = true;
+            wake_up_interruptible(&ctnx->wait);
Can you please explain more on what exactly we are trying to do here (may be you can list down the
call flow)? I am not sure if i understood this correctly. But why would you need to wakeup the thread here? We are here because thread is already woken up. mmc_wait_for_data_req_done() would return only if either the new request is received or the currently running request on controller is completed. This problem is produced when card is removed and running some operation.
If card is removed, mmc_wait_for_data_req_done() is returned error, right?
But if there was async request and running async operation, then maybe run mmc_start_request() with areq.
controller is waiting "_for_req_data_done_" for areq.
Quoted 3 lines
If request is host->areq.process is running at this time.
But if request is areq(prepared request), then mmc_wait_for_data_req_done should be pending. and system is dead-lock.
I also found this problem with sdcard.
I think it can be solved easily. Did you see the following patch? [PATCH 1/2] mmc: core: fix permanent sleep of mmcqd during card removal We may just get the hint from __mmc_start_req function.
Thanks,
Seungwon Jeon
Quoted 19 lines
Best Regards,
Jaehoon Chung
Quoted 9 lines
Regards,
Subhash
drivers/mmc/core/core.c struct mmc_async_req *mmc_start_req(struct mmc_host *host, · line 540
Quoted 4 lines
drivers/mmc/core/core.c +1 -0
+        }
          /*
           * Check BKOPS urgency for each R1 response
           */
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html