* [PATCH] mmc: align max segment size with logical block size
@ 2019-02-27 13:02 Ming Lei
2019-02-27 13:57 ` Ulf Hansson
0 siblings, 1 reply; 3+ messages in thread
From: Ming Lei @ 2019-02-27 13:02 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Ming Lei, Naresh Kamboju, Faiz Abbas, linux-block
Logical block size is the lowest possible block size that the storage
device can address. Max segment size is often related with controller's
DMA capability. And it is reasonable to align max segment size with
logical block size.
SDHCI sets un-aligned max segment size, and causes ADMA error, so
fix it by aligning max segment size with logical block size.
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: linux-block@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/mmc/core/block.c | 6 ------
drivers/mmc/core/queue.c | 9 ++++++++-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 14f3fdb8c6bb..9ce8eb51a60f 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2380,12 +2380,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
"mmcblk%u%s", card->host->index, subname ? subname : "");
- if (mmc_card_mmc(card))
- blk_queue_logical_block_size(md->queue.queue,
- card->ext_csd.data_sector_size);
- else
- blk_queue_logical_block_size(md->queue.queue, 512);
-
set_capacity(md->disk, size);
if (mmc_host_cmd23(card->host)) {
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index cc19e71c71d4..891bdbe14192 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -355,6 +355,7 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
{
struct mmc_host *host = card->host;
u64 limit = BLK_BOUNCE_HIGH;
+ unsigned block_size = 512;
if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask)
limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
@@ -368,7 +369,13 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
blk_queue_max_hw_sectors(mq->queue,
min(host->max_blk_count, host->max_req_size / 512));
blk_queue_max_segments(mq->queue, host->max_segs);
- blk_queue_max_segment_size(mq->queue, host->max_seg_size);
+
+ if (mmc_card_mmc(card))
+ block_size = card->ext_csd.data_sector_size;
+
+ blk_queue_logical_block_size(mq->queue, block_size);
+ blk_queue_max_segment_size(mq->queue, host->max_seg_size &
+ ~(block_size - 1));
INIT_WORK(&mq->recovery_work, mmc_mq_recovery_handler);
INIT_WORK(&mq->complete_work, mmc_blk_mq_complete_work);
--
2.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mmc: align max segment size with logical block size
2019-02-27 13:02 [PATCH] mmc: align max segment size with logical block size Ming Lei
@ 2019-02-27 13:57 ` Ulf Hansson
2019-02-27 15:50 ` Ming Lei
0 siblings, 1 reply; 3+ messages in thread
From: Ulf Hansson @ 2019-02-27 13:57 UTC (permalink / raw)
To: Ming Lei
Cc: linux-mmc, Naresh Kamboju, Faiz Abbas, linux-block, Christoph Hellwig
+ Christoph
On Wed, 27 Feb 2019 at 14:02, Ming Lei <ming.lei@redhat.com> wrote:
>
> Logical block size is the lowest possible block size that the storage
> device can address. Max segment size is often related with controller's
> DMA capability. And it is reasonable to align max segment size with
> logical block size.
>
> SDHCI sets un-aligned max segment size, and causes ADMA error, so
> fix it by aligning max segment size with logical block size.
Seems reasonable. However, is this specific to MMC or should it be
managed in block layer?
>
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
> Cc: Faiz Abbas <faiz_abbas@ti.com>
> Cc: linux-block@vger.kernel.org
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> drivers/mmc/core/block.c | 6 ------
> drivers/mmc/core/queue.c | 9 ++++++++-
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 14f3fdb8c6bb..9ce8eb51a60f 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2380,12 +2380,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
> snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
> "mmcblk%u%s", card->host->index, subname ? subname : "");
>
> - if (mmc_card_mmc(card))
> - blk_queue_logical_block_size(md->queue.queue,
> - card->ext_csd.data_sector_size);
> - else
> - blk_queue_logical_block_size(md->queue.queue, 512);
> -
> set_capacity(md->disk, size);
>
> if (mmc_host_cmd23(card->host)) {
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> index cc19e71c71d4..891bdbe14192 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -355,6 +355,7 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
> {
> struct mmc_host *host = card->host;
> u64 limit = BLK_BOUNCE_HIGH;
> + unsigned block_size = 512;
>
> if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask)
> limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
> @@ -368,7 +369,13 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
> blk_queue_max_hw_sectors(mq->queue,
> min(host->max_blk_count, host->max_req_size / 512));
> blk_queue_max_segments(mq->queue, host->max_segs);
> - blk_queue_max_segment_size(mq->queue, host->max_seg_size);
> +
> + if (mmc_card_mmc(card))
> + block_size = card->ext_csd.data_sector_size;
> +
> + blk_queue_logical_block_size(mq->queue, block_size);
> + blk_queue_max_segment_size(mq->queue, host->max_seg_size &
> + ~(block_size - 1));
Can't we use round_down() instead? No?
>
> INIT_WORK(&mq->recovery_work, mmc_mq_recovery_handler);
> INIT_WORK(&mq->complete_work, mmc_blk_mq_complete_work);
> --
> 2.9.5
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mmc: align max segment size with logical block size
2019-02-27 13:57 ` Ulf Hansson
@ 2019-02-27 15:50 ` Ming Lei
0 siblings, 0 replies; 3+ messages in thread
From: Ming Lei @ 2019-02-27 15:50 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Naresh Kamboju, Faiz Abbas, linux-block, Christoph Hellwig
On Wed, Feb 27, 2019 at 02:57:04PM +0100, Ulf Hansson wrote:
> + Christoph
>
> On Wed, 27 Feb 2019 at 14:02, Ming Lei <ming.lei@redhat.com> wrote:
> >
> > Logical block size is the lowest possible block size that the storage
> > device can address. Max segment size is often related with controller's
> > DMA capability. And it is reasonable to align max segment size with
> > logical block size.
> >
> > SDHCI sets un-aligned max segment size, and causes ADMA error, so
> > fix it by aligning max segment size with logical block size.
>
> Seems reasonable. However, is this specific to MMC or should it be
> managed in block layer?
So far, only see such broken max segment size on MMC, so suggest to
fix in MMC, otherwise we may have to align the limit in fast path.
>
> >
> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> > Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
> > Cc: Faiz Abbas <faiz_abbas@ti.com>
> > Cc: linux-block@vger.kernel.org
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> > drivers/mmc/core/block.c | 6 ------
> > drivers/mmc/core/queue.c | 9 ++++++++-
> > 2 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index 14f3fdb8c6bb..9ce8eb51a60f 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -2380,12 +2380,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
> > snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
> > "mmcblk%u%s", card->host->index, subname ? subname : "");
> >
> > - if (mmc_card_mmc(card))
> > - blk_queue_logical_block_size(md->queue.queue,
> > - card->ext_csd.data_sector_size);
> > - else
> > - blk_queue_logical_block_size(md->queue.queue, 512);
> > -
> > set_capacity(md->disk, size);
> >
> > if (mmc_host_cmd23(card->host)) {
> > diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> > index cc19e71c71d4..891bdbe14192 100644
> > --- a/drivers/mmc/core/queue.c
> > +++ b/drivers/mmc/core/queue.c
> > @@ -355,6 +355,7 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
> > {
> > struct mmc_host *host = card->host;
> > u64 limit = BLK_BOUNCE_HIGH;
> > + unsigned block_size = 512;
> >
> > if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask)
> > limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
> > @@ -368,7 +369,13 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
> > blk_queue_max_hw_sectors(mq->queue,
> > min(host->max_blk_count, host->max_req_size / 512));
> > blk_queue_max_segments(mq->queue, host->max_segs);
> > - blk_queue_max_segment_size(mq->queue, host->max_seg_size);
> > +
> > + if (mmc_card_mmc(card))
> > + block_size = card->ext_csd.data_sector_size;
> > +
> > + blk_queue_logical_block_size(mq->queue, block_size);
> > + blk_queue_max_segment_size(mq->queue, host->max_seg_size &
> > + ~(block_size - 1));
>
> Can't we use round_down() instead? No?
Sure, use round_down() in V2.
thanks,
Ming
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-27 15:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-27 13:02 [PATCH] mmc: align max segment size with logical block size Ming Lei
2019-02-27 13:57 ` Ulf Hansson
2019-02-27 15:50 ` Ming Lei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).