Skip to contentSkip to conversation

mmc, block: override default max_dev_sectors

PATCH In review Pawel Wodkowski 4 participants 6 messages Mar 2, 2016 – Mar 5, 2016

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover mmc, block: override default max_dev_sectors

From: Mateusz Nowak <[email protected]>

commit ca369d51b3e1 ("block/sd: Fix device-imposed transfer length limits") introduced max_dev_sectors as upper limit for max_hw_sectors. This caused limitation of maximum block size to default max_dev_sectors value - 127, causing performance degradation with higher block sizes.

This patchset is allowing mmc driver to override max_dev_sectors value.

Cc: [email protected]
Mateusz Nowak (2):
block: Allow to set max_dev_sectors for a queue. mmc/block: Override default max_dev_sectors.

 block/blk-settings.c     | 24 ++++++++++++++++++++++++
 drivers/mmc/card/queue.c | 10 ++++++++++
 include/linux/blkdev.h   |  1 +
 3 files changed, 35 insertions(+)
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/2 block: Allow to set max_dev_sectors for a queue.

From: Mateusz Nowak <[email protected]>

max_dev_sectors is the upper limit for supported block size by the block device. Since it is used for limiting up max_hw_sectors value (and, as consequence maximum block size for request), lower level driver may need to set it to non-default value.
This patch implements function allowing to do so.

Fixes ca369d51b3e1 ("block/sd: Fix device-imposed transfer length limits")
---
 block/blk-settings.c   | 24 ++++++++++++++++++++++++
 include/linux/blkdev.h |  1 +
 2 files changed, 25 insertions(+)
block/blk-settings.c +24 -0
index c7bb666aafd1..276920dfacd0 100644
@@ -253,6 +253,30 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto
 EXPORT_SYMBOL(blk_queue_max_hw_sectors);
 
 /**
+ * blk_queue_max_dev_sectors - set max sectors supported by a device for
+ * this queue.
+ * @q:	the request queue for the device
+ * @max_dev_sectors:	max device sectors in the usual 512b unit
+ *
+ * Description:
+ *    Enables a low level driver to set a device supported upper limit,
+ *    max_dev_sectors, on the size of request. max_dev_sectors is set by
+ *    the device driver based upon the capabilities of the device.
+ *
+ *    max_dev_sectors is upper limit for max_hw_sectors value, thus
+ *    it should be set before changing the latter value.
+ **/
+void blk_queue_max_dev_sectors(struct request_queue *q,
+		unsigned int max_dev_sectors)
+{
+	struct queue_limits *limits = &q->limits;
+
+	limits->max_dev_sectors = min_t(unsigned int, max_dev_sectors,
+					BLK_DEF_MAX_SECTORS);
+}
+EXPORT_SYMBOL(blk_queue_max_dev_sectors);
+
+/**
  * blk_queue_chunk_sectors - set size of the chunk for this queue
  * @q:  the request queue for the device
  * @chunk_sectors:  chunk sectors in the usual 512b unit
include/linux/blkdev.h +1 -0
index 4571ef1a12a9..2752d3afcf1c 100644
@@ -967,6 +967,7 @@ extern void blk_cleanup_queue(struct request_queue *);
 extern void blk_queue_make_request(struct request_queue *, make_request_fn *);
 extern void blk_queue_bounce_limit(struct request_queue *, u64);
 extern void blk_queue_max_hw_sectors(struct request_queue *, unsigned int);
+extern void blk_queue_max_dev_sectors(struct request_queue *, unsigned int);
 extern void blk_queue_chunk_sectors(struct request_queue *, unsigned int);
 extern void blk_queue_max_segments(struct request_queue *, unsigned short);
 extern void blk_queue_max_segment_size(struct request_queue *, unsigned int);
Signature · 1 line
1.9.1
#2 lore raw

PATCH 2/2 mmc/block: Override default max_dev_sectors.

In reply to Pawel Wodkowski

From: Mateusz Nowak <[email protected]>

Fixes ca369d51b3e1 ("block/sd: Fix device-imposed transfer length limits")

introduced max_dev_sectors as upper limit for max_hw_sectors. This caused limitation of maximum block size to default max_dev_sectors value - 127, causing performance degradation with higher block sizes.

This patch is overriding max_dev_sectors value, so it will be set to maximum possible value to be used with set block count command, so 0xffff for eMMC cards, and 0xffffffff for SD cards.

Fixes ca369d51b3e1 ("block/sd: Fix device-imposed transfer length limits")
---
 drivers/mmc/card/queue.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/mmc/card/queue.c +10 -0
index 6f4323c6d653..9175b8cd54de 100644
@@ -212,6 +212,16 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
 	if (mmc_can_erase(card))
 		mmc_queue_setup_discard(mq->queue, card);
 
+	/*
+	 * Set maximum block count accepted by the card.
+	 * For eMMC, this can be 16 bit value,
+	 * for SD, this can be 32 bit value
+	 */
+	if (mmc_card_mmc(card))
+		blk_queue_max_dev_sectors(mq->queue, 0xffff);
+	else if (mmc_card_sd(card))
+		blk_queue_max_dev_sectors(mq->queue, 0xffffffff);
+
 #ifdef CONFIG_MMC_BLOCK_BOUNCE
 	if (host->max_segs == 1) {
 		unsigned int bouncesz;
Signature · 1 line
1.9.1
#3 lore raw
In reply to Pawel Wodkowski
Quoted 8 lines · Pawel Wodkowski

From: Mateusz Nowak <[email protected]>

commit ca369d51b3e1 ("block/sd: Fix device-imposed transfer length limits") introduced max_dev_sectors as upper limit for max_hw_sectors. This caused limitation of maximum block size to default max_dev_sectors value - 127, causing performance degradation with higher block sizes.

This patchset is allowing mmc driver to override max_dev_sectors value.

Past commit 5f009d ("block: Initialize max_dev_sectors to 0"), this should not be needed.

#4 lore raw

Hi,

Quoted 15 lines

-----Original Message-----
From: Christoph Hellwig [mailto:[email protected]]
Sent: Wednesday, March 2, 2016 18:12
To: Wodkowski, PawelX <[email protected]>
Cc: [email protected]; Nowak, Mateusz <[email protected]>; [email protected]
Subject: Re: [PATCH 0/2] mmc, block: override default max_dev_sectors

Quoted 6 lines · Pawel Wodkowski

From: Mateusz Nowak <[email protected]>

commit ca369d51b3e1 ("block/sd: Fix device-imposed transfer length limits") introduced max_dev_sectors as upper limit for max_hw_sectors. This caused limitation of maximum block size to default max_dev_sectors value - 127, causing performance degradation with

Quoted 6 lines

higher block sizes.

Quoted 2 lines

This patchset is allowing mmc driver to override max_dev_sectors value.

Past commit 5f009d ("block: Initialize max_dev_sectors to 0"), this should not be needed.

I missed the fix you mention while rebasing from rc3. Thus apologies for providing incorrectly described and not necessarily needed patch set.

However, since some drivers may still want to override max_dev_sectors, would the patches be good to be redone and resubmitted as improvement? If so, please let me know if there are any issues with them (apart from incorrect description), so their next version can be of improved quality.

Best regards,
Mateusz.
--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

#5 lore raw
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Mateusz" == Nowak, Mateusz <[email protected]> writes:

Past commit 5f009d ("block: Initialize max_dev_sectors to 0"), this should not be needed.

Mateusz> I missed the fix you mention while rebasing from rc3. Thus Mateusz> apologies for providing incorrectly described and not Mateusz> necessarily needed patch set.

Mateusz> However, since some drivers may still want to override Mateusz> max_dev_sectors, would the patches be good to be redone and Mateusz> resubmitted as improvement?

Nothing prevents you from setting max_dev_sectors in the driver. That's why it's there. The default being set in my original patch just had the unfortunate side-effect of clamping the controller limit.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering