Skip to contentSkip to conversation

mmc: set max_discard_sectors value for mmc queue

PATCH In review Chuanxiao Dong 3 participants 7 messages Feb 12, 2011 – Feb 14, 2011 MMC, SD and SDIO

lore
Share & tools
lore
#0 lore raw

PATCH v4 1/3 mmc: set max_discard_sectors value for mmc queue

max_discard_sectors value is UINT_MAX which means kernel block layer can pass down unlimited sectors to MMC driver to erase. But erasing so many sectors may delay some other important I/O requests. This is not preferred.

So use 'pref_erase' to set a suitable max_discard_sectors value for mmc queue to avoid erasing too many sectors at one time.

---
 drivers/mmc/card/queue.c |    3 ++-
 drivers/mmc/core/core.c  |   20 ++++++++++++++++++++
 include/linux/mmc/core.h |    1 +
 3 files changed, 23 insertions(+), 1 deletions(-)
drivers/mmc/card/queue.c +2 -1
index 4e42d03..6c13859 100644
@@ -131,7 +131,8 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card, spinlock_t *lock
 	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, mq->queue);
 	if (mmc_can_erase(card)) {
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mq->queue);
-		mq->queue->limits.max_discard_sectors = UINT_MAX;
+		mq->queue->limits.max_discard_sectors =
+			mmc_set_max_discard_sectors(card);
 		if (card->erased_byte == 0)
 			mq->queue->limits.discard_zeroes_data = 1;
 		if (!mmc_can_trim(card) && is_power_of_2(card->erase_size)) {
drivers/mmc/core/core.c +20 -0
index 34a7e8c..0eb27aa 100644
@@ -1470,6 +1470,26 @@ int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
 }
 EXPORT_SYMBOL(mmc_erase_group_aligned);
 
+/*
+ * Set max_discard_sectors for mmc queue.
+ * max_discard_sectors will determine how many sectors can be erased at one
+ * time. Sometimes user may want to erase a large area of SD or MMC card, it may
+ * take long time which delay some other important I/O requests. So we would
+ * better set some values for max_discard_sectors which can forbid block layer
+ * pass too many sectors at one time.
+ *
+ * 'pref_erase' is initialized for this purpose. We can use it to set a preferred
+ * value for it.
+ */
+int mmc_set_max_discard_sectors(struct mmc_card *card)
+{
+	if (card->pref_erase)
+		return card->pref_erase;
+	else
+		return UINT_MAX;
+}
+EXPORT_SYMBOL(mmc_set_max_discard_sectors);
+
 int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen)
 {
 	struct mmc_command cmd;
include/linux/mmc/core.h +1 -0
index 64e013f..a218d57 100644
@@ -152,6 +152,7 @@ extern int mmc_can_trim(struct mmc_card *card);
 extern int mmc_can_secure_erase_trim(struct mmc_card *card);
 extern int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
 				   unsigned int nr);
+extern int mmc_set_max_discard_sectors(struct mmc_card *card);
 
 extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
 
Signature · 1 line
1.6.6.1
#1 lore raw
Quoted 8 lines · On Saturday 12 February 2011 07:22:14 Chuanxiao Dong wrote:

max_discard_sectors value is UINT_MAX which means kernel block layer can pass down unlimited sectors to MMC driver to erase. But erasing so many sectors may delay some other important I/O requests. This is not preferred.

So use 'pref_erase' to set a suitable max_discard_sectors value for mmc queue to avoid erasing too many sectors at one time.

I'm not sure about this one. pref_erase on SDHC cards should be the *minimum* unit you can erase in one request, not the maximum. Erasing an arbitrary number of allocation units on an SDHC card should complete almost instantly, because it only needs to update a single table with the allocation units.

Discarding partial allocation units will take a lot longer, because the card then has to copy over the remaining blocks.

Arnd

#2 lore raw
Quoted 13 lines

-----Original Message-----
From: Arnd Bergmann [mailto:[email protected]]
Sent: Saturday, February 12, 2011 4:38 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v4 1/3]mmc: set max_discard_sectors value for mmc queue

Quoted 4 lines · On Saturday 12 February 2011 07:22:14 Chuanxiao Dong wrote:

max_discard_sectors value is UINT_MAX which means kernel block layer can pass down unlimited sectors to MMC driver to erase. But erasing so many sectors may delay some other important I/O requests. This is not preferred.

Quoted 7 lines
Quoted 4 lines

So use 'pref_erase' to set a suitable max_discard_sectors value for mmc queue to avoid erasing too many sectors at one time.

I'm not sure about this one. pref_erase on SDHC cards should be the *minimum* unit you can erase in one request, not the maximum. Erasing an arbitrary

Hi Arnd,
I found the comment in mmc_init_erase() function which explains the 'pref_erase':
'pref_erase' is defined as a guide to limit erases to that size and alignment.
So I think it is not the minimum unit driver can erase, also not the maximum erase unit. It just a guide erase size for driver which can avoid holding host controller too long to response other I/O requests.

Quoted 5 lines

number of allocation units on an SDHC card should complete almost instantly, because it only needs to update a single table with the allocation units.

Discarding partial allocation units will take a lot longer, because the card then has to copy over the remaining blocks.

Is discarding started when card is idle or right after erase command? Can it cause some other I/O requests delayed?

#3 lore raw
Quoted 11 lines · Chuanxiao
Quoted 11 lines

From: Arnd Bergmann [mailto:[email protected]]

Quoted 8 lines · On Saturday 12 February 2011 07:22:14 Chuanxiao Dong wrote:

max_discard_sectors value is UINT_MAX which means kernel block layer can pass down unlimited sectors to MMC driver to erase. But erasing so many sectors may delay some other important I/O requests. This is not preferred.

So use 'pref_erase' to set a suitable max_discard_sectors value for mmc queue to avoid erasing too many sectors at one time.

Quoted 8 lines
Quoted 2 lines

I'm not sure about this one. pref_erase on SDHC cards should be the *minimum* unit you can erase in one request, not the maximum. Erasing an arbitrary

I found the comment in mmc_init_erase() function which explains the 'pref_erase':
'pref_erase' is defined as a guide to limit erases to that size and alignment. So I think it is not the minimum unit driver can erase, also not the maximum erase unit. It just a guide erase size for driver which can avoid holding host controller too long to response other I/O requests.

Ok, I see. Adrian introduced the function and the comment last year, maybe he can comment on this some more. For all cards I've seen, multi-AU erases on SDHC cards are really fast, though the standard clearly allows for very slow erases.

My feeling is that the function is suboptimal right now, because it assumes that one AU is the best size. If a card reports that it can erase many AUs quickly (large N_erase, small T_erase, relatively large T_offset), we should report that to the user.

Quoted 8 lines
Quoted 5 lines

number of allocation units on an SDHC card should complete almost instantly, because it only needs to update a single table with the allocation units.

Discarding partial allocation units will take a lot longer, because the card then has to copy over the remaining blocks.

Is discarding started when card is idle or right after erase command? Can it cause some other I/O requests delayed?

As far as I can tell, most operations on the card are synchronous when it's writing. The erase command on a partial AU should not return until the remaining data is copied to a new location, so it will take some time, but physically the AU can be done in the background, if reading from it simply returns a zero buffer rather than accessing the old data. Write-after-erase might have to wait for the erase to complete, but good cards could also hide that.

Arnd

#4 lore raw
Quoted 53 lines

-----Original Message-----
From: Arnd Bergmann [mailto:[email protected]]
Sent: Sunday, February 13, 2011 2:05 AM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v4 1/3]mmc: set max_discard_sectors value for mmc queue

Quoted 3 lines · Chuanxiao
Quoted 3 lines

From: Arnd Bergmann [mailto:[email protected]]

Quoted 1 line · On Saturday 12 February 2011 07:22:14 Chuanxiao Dong wrote:

max_discard_sectors value is UINT_MAX which means kernel block layer can

pass

Quoted 1 line
Quoted 1 line
Quoted 1 line

down unlimited sectors to MMC driver to erase. But erasing so many sectors

may

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

delay some other important I/O requests. This is not preferred.

So use 'pref_erase' to set a suitable max_discard_sectors value for mmc

queue to

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

avoid erasing too many sectors at one time.

I'm not sure about this one. pref_erase on SDHC cards should be the

*minimum*

Quoted 3 lines
Quoted 1 line

unit you can erase in one request, not the maximum. Erasing an arbitrary

I found the comment in mmc_init_erase() function which explains the

'pref_erase':

Quoted 4 lines

'pref_erase' is defined as a guide to limit erases to that size and alignment. So I think it is not the minimum unit driver can erase, also not the maximum erase unit. It just a guide erase size for driver which can avoid holding host controller too long to response other I/O requests.

Ok, I see. Adrian introduced the function and the comment last year, maybe he can comment on this some more. For all cards I've seen, multi-AU erases on SDHC cards are really fast, though the standard clearly allows for very slow erases.

My feeling is that the function is suboptimal right now, because it assumes that one AU is the best size. If a card reports that it can erase many AUs quickly (large N_erase, small T_erase, relatively large T_offset), we should report that to the user.

Quoted 8 lines
Quoted 5 lines

number of allocation units on an SDHC card should complete almost instantly, because it only needs to update a single table with the allocation units.

Discarding partial allocation units will take a lot longer, because the card then has to copy over the remaining blocks.

Is discarding started when card is idle or right after erase command? Can it cause some other I/O requests delayed?

Quoted 7 lines

As far as I can tell, most operations on the card are synchronous when it's writing. The erase command on a partial AU should not return until the remaining data is copied to a new location, so it will take some time, but physically the AU can be done in the background, if reading from it simply returns a zero buffer rather than accessing the old data. Write-after-erase might have to wait for the erase to complete, but good cards could also hide that.

Thanks, Arnd.
When I do trim with a 32GB eMMC card in my platform, sometimes I can get the 10s timeout errors but sometimes not. I am not much clear about the "discarding partial AU will take a lot longer". If this action is hide for driver, then I think from driver side, the UINT_MAX value for max_discard_sectors will be OK. But if this action sometimes need driver to wait for some hardware interrupt, then I think the UINT_MAX value is not preferred. Arnd, have any suggestion of dealing this? What I thought is using other value instead of using UINT_MAX.

Thanks
Chuanxiao

#5 lore raw
Quoted 9 lines · Chuanxiao

When I do trim with a 32GB eMMC card in my platform, sometimes I can get the 10s timeout errors but sometimes not. I am not much clear about the "discarding partial AU will take a lot longer". If this action is hide for driver, then I think from driver side, the UINT_MAX value for max_discard_sectors will be OK. But if this action sometimes need driver to wait for some hardware interrupt, then I think the UINT_MAX value is not preferred.

Arnd, have any suggestion of dealing this? What I thought is using other value instead of using UINT_MAX.

I'm not too familiar with the eMMC spec, but it should have a way to calculate a maximum trim timeout like SD 3.0 does for AU erases. When I've seen the timeouts with SDHCI (missing your patch), it was always a bug in the driver, and the erase was already completed before the driver even started waiting for the interrupt.

10 seconds still sounds like a reasonable timeout, and we should probably not issue any requests that might take longer than that, so I think the interesting question is how to determine a good value for pref_erase, so we can still take your patch.

Arnd

#6 lore raw
In reply to Chuanxiao Dong
Quoted 41 lines · ext Chuanxiao Dong

max_discard_sectors value is UINT_MAX which means kernel block layer can pass down unlimited sectors to MMC driver to erase. But erasing so many sectors may delay some other important I/O requests. This is not preferred.

So use 'pref_erase' to set a suitable max_discard_sectors value for mmc queue to avoid erasing too many sectors at one time.

---
  drivers/mmc/card/queue.c |    3 ++-
  drivers/mmc/core/core.c  |   20 ++++++++++++++++++++
  include/linux/mmc/core.h |    1 +
  3 files changed, 23 insertions(+), 1 deletions(-)
drivers/mmc/card/queue.c +2 -1
index 4e42d03..6c13859 100644
@@ -131,7 +131,8 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card, spinlock_t *lock
  	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, mq->queue);
  	if (mmc_can_erase(card)) {
  		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mq->queue);
-		mq->queue->limits.max_discard_sectors = UINT_MAX;
+		mq->queue->limits.max_discard_sectors =
+			mmc_set_max_discard_sectors(card);
  		if (card->erased_byte == 0)
  			mq->queue->limits.discard_zeroes_data = 1;
  		if (!mmc_can_trim(card)&&  is_power_of_2(card->erase_size)) {
drivers/mmc/core/core.c +5 -0
index 34a7e8c..0eb27aa 100644
@@ -1470,6 +1470,26 @@ int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
  }
  EXPORT_SYMBOL(mmc_erase_group_aligned);
 
+/*
+ * Set max_discard_sectors for mmc queue.
+ * max_discard_sectors will determine how many sectors can be erased at one
+ * time. Sometimes user may want to erase a large area of SD or MMC card, it may
+ * take long time which delay some other important I/O requests. So we would

drivers/mmc/core/core.c int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from, · line 1487

Quoted 10 lines
drivers/mmc/core/core.c +10 -0
+ * better set some values for max_discard_sectors which can forbid block layer
+ * pass too many sectors at one time.
+ *
+ * 'pref_erase' is initialized for this purpose. We can use it to set a preferred
+ * value for it.
+ */
+int mmc_set_max_discard_sectors(struct mmc_card *card)
+{
+	if (card->pref_erase)
+		return card->pref_erase;

This will cause orders of magnitude decrease in secure erase performance for eMMC.

Quoted 20 lines
drivers/mmc/core/core.c +5 -0
+	else
+		return UINT_MAX;
+}
+EXPORT_SYMBOL(mmc_set_max_discard_sectors);
+
  int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen)
  {
  	struct mmc_command cmd;
include/linux/mmc/core.h +1 -0
index 64e013f..a218d57 100644
@@ -152,6 +152,7 @@ extern int mmc_can_trim(struct mmc_card *card);
  extern int mmc_can_secure_erase_trim(struct mmc_card *card);
  extern int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
  				   unsigned int nr);
+extern int mmc_set_max_discard_sectors(struct mmc_card *card);
 
  extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);