diff mbox series

[1/5] mmc: core: Delete bounce buffer Kconfig option

Message ID 20170510082418.10513-2-linus.walleij@linaro.org
State New
Headers show
Series mmc: core: modernize ioctl() requests | expand

Commit Message

Linus Walleij May 10, 2017, 8:24 a.m. UTC
This option is activated by all multiplatform configs and what
not so we almost always have it turned on, and the memory it
saves is negligible, even more so moving forward. The actual
bounce buffer only gets allocated only when used, the only
thing the ifdefs are saving is a little bit of code.

It is highly improper to have this as a Kconfig option that
get turned on by Kconfig, make this a pure runtime-thing and
let the host decide whether we use bounce buffers. We add a
new property "disable_bounce" to the host struct.

Notice that mmc_queue_calc_bouncesz() already disables the
bounce buffers if host->max_segs != 1, so any arch that has a
maximum number of segments higher than 1 will have bounce
buffers disabled.

The option CONFIG_MMC_BLOCK_BOUNCE is default y so the
majority of platforms in the kernel already have it on, and
it then gets turned off at runtime since most of these have
a host->max_segs > 1. The few exceptions that have
host->max_segs == 1 and still turn off the bounce buffering
are those that disable it in their defconfig.

Those are the following:

arch/arm/configs/colibri_pxa300_defconfig
arch/arm/configs/zeus_defconfig
- Uses MMC_PXA, drivers/mmc/host/pxamci.c
- Sets host->max_segs = NR_SG, which is 1
- This needs its bounce buffer deactivated so we set
  host->disable_bounce to true in the host driver

arch/arm/configs/davinci_all_defconfig
- Uses MMC_DAVINCI, drivers/mmc/host/davinci_mmc.c
- This driver sets host->max_segs to MAX_NR_SG, which is 16
- That means this driver anyways disabled bounce buffers
- No special action needed for this platform

arch/arm/configs/lpc32xx_defconfig
arch/arm/configs/nhk8815_defconfig
arch/arm/configs/u300_defconfig
- Uses MMC_ARMMMCI, drivers/mmc/host/mmci.[c|h]
- This driver by default sets host->max_segs to NR_SG,
  which is 128, unless a DMA engine is used, and in that case
  the number of segments are also > 1
- That means this driver already disables bounce buffers
- No special action needed for these platforms

arch/arm/configs/sama5_defconfig
- Uses MMC_SDHCI, MMC_SDHCI_PLTFM, MMC_SDHCI_OF_AT91, MMC_ATMELMCI
- Uses drivers/mmc/host/sdhci.c
- Normally sets host->max_segs to SDHCI_MAX_SEGS which is 128 and
  thus disables bounce buffers
- Sets host->max_segs to 1 if SDHCI_USE_SDMA is set
- SDHCI_USE_SDMA is only set by SDHCI on PCI adapers
- That means that for this platform bounce buffers are already
  disabled at runtime
- No special action needed for this platform

arch/blackfin/configs/CM-BF533_defconfig
arch/blackfin/configs/CM-BF537E_defconfig
- Uses MMC_SPI (a simple MMC card connected on SPI pins)
- Uses drivers/mmc/host/mmc_spi.c
- Sets host->max_segs to MMC_SPI_BLOCKSATONCE which is 128
- That means this platform already disables bounce buffers at
  runtime
- No special action needed for these platforms

arch/mips/configs/cavium_octeon_defconfig
- Uses MMC_CAVIUM_OCTEON, drivers/mmc/host/cavium.c
- Sets host->max_segs to 16 or 1
- Setting host->disable_bounce to be sure for the 1 case

arch/mips/configs/qi_lb60_defconfig
- Uses MMC_JZ4740, drivers/mmc/host/jz4740_mmc.c
- This sets host->max_segs to 128 so bounce buffers are
  already runtime disabled
- No action needed for this platform

It would be interesting to come up with a list of the platforms
that actually end up using bounce buffers. I have not been
able to infer such a list, but it occurs when
host->max_segs == 1 and the bounce buffering is not explicitly
disabled.

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

---
 drivers/mmc/core/Kconfig  | 18 ------------------
 drivers/mmc/core/queue.c  | 15 +--------------
 drivers/mmc/host/cavium.c |  3 +++
 drivers/mmc/host/pxamci.c |  6 ++++++
 include/linux/mmc/host.h  |  1 +
 5 files changed, 11 insertions(+), 32 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

Ulf Hansson May 15, 2017, 11:55 a.m. UTC | #1
On 10 May 2017 at 10:24, Linus Walleij <linus.walleij@linaro.org> wrote:
> This option is activated by all multiplatform configs and what

> not so we almost always have it turned on, and the memory it

> saves is negligible, even more so moving forward. The actual

> bounce buffer only gets allocated only when used, the only

> thing the ifdefs are saving is a little bit of code.

>

> It is highly improper to have this as a Kconfig option that

> get turned on by Kconfig, make this a pure runtime-thing and

> let the host decide whether we use bounce buffers. We add a

> new property "disable_bounce" to the host struct.

>

> Notice that mmc_queue_calc_bouncesz() already disables the

> bounce buffers if host->max_segs != 1, so any arch that has a

> maximum number of segments higher than 1 will have bounce

> buffers disabled.

>

> The option CONFIG_MMC_BLOCK_BOUNCE is default y so the

> majority of platforms in the kernel already have it on, and

> it then gets turned off at runtime since most of these have

> a host->max_segs > 1. The few exceptions that have

> host->max_segs == 1 and still turn off the bounce buffering

> are those that disable it in their defconfig.

>

> Those are the following:

>

> arch/arm/configs/colibri_pxa300_defconfig

> arch/arm/configs/zeus_defconfig

> - Uses MMC_PXA, drivers/mmc/host/pxamci.c

> - Sets host->max_segs = NR_SG, which is 1

> - This needs its bounce buffer deactivated so we set

>   host->disable_bounce to true in the host driver

>

> arch/arm/configs/davinci_all_defconfig

> - Uses MMC_DAVINCI, drivers/mmc/host/davinci_mmc.c

> - This driver sets host->max_segs to MAX_NR_SG, which is 16

> - That means this driver anyways disabled bounce buffers

> - No special action needed for this platform

>

> arch/arm/configs/lpc32xx_defconfig

> arch/arm/configs/nhk8815_defconfig

> arch/arm/configs/u300_defconfig

> - Uses MMC_ARMMMCI, drivers/mmc/host/mmci.[c|h]

> - This driver by default sets host->max_segs to NR_SG,

>   which is 128, unless a DMA engine is used, and in that case

>   the number of segments are also > 1

> - That means this driver already disables bounce buffers

> - No special action needed for these platforms

>

> arch/arm/configs/sama5_defconfig

> - Uses MMC_SDHCI, MMC_SDHCI_PLTFM, MMC_SDHCI_OF_AT91, MMC_ATMELMCI

> - Uses drivers/mmc/host/sdhci.c

> - Normally sets host->max_segs to SDHCI_MAX_SEGS which is 128 and

>   thus disables bounce buffers

> - Sets host->max_segs to 1 if SDHCI_USE_SDMA is set

> - SDHCI_USE_SDMA is only set by SDHCI on PCI adapers

> - That means that for this platform bounce buffers are already

>   disabled at runtime

> - No special action needed for this platform

>

> arch/blackfin/configs/CM-BF533_defconfig

> arch/blackfin/configs/CM-BF537E_defconfig

> - Uses MMC_SPI (a simple MMC card connected on SPI pins)

> - Uses drivers/mmc/host/mmc_spi.c

> - Sets host->max_segs to MMC_SPI_BLOCKSATONCE which is 128

> - That means this platform already disables bounce buffers at

>   runtime

> - No special action needed for these platforms

>

> arch/mips/configs/cavium_octeon_defconfig

> - Uses MMC_CAVIUM_OCTEON, drivers/mmc/host/cavium.c

> - Sets host->max_segs to 16 or 1

> - Setting host->disable_bounce to be sure for the 1 case

>

> arch/mips/configs/qi_lb60_defconfig

> - Uses MMC_JZ4740, drivers/mmc/host/jz4740_mmc.c

> - This sets host->max_segs to 128 so bounce buffers are

>   already runtime disabled

> - No action needed for this platform

>

> It would be interesting to come up with a list of the platforms

> that actually end up using bounce buffers. I have not been

> able to infer such a list, but it occurs when

> host->max_segs == 1 and the bounce buffering is not explicitly

> disabled.


Thanks for the descriptive change-log!

[...]

> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h

> index 21385ac0c9b1..b53c0e18a33b 100644

> --- a/include/linux/mmc/host.h

> +++ b/include/linux/mmc/host.h

> @@ -310,6 +310,7 @@ struct mmc_host {

>         /* host specific block data */

>         unsigned int            max_seg_size;   /* see blk_queue_max_segment_size */

>         unsigned short          max_segs;       /* see blk_queue_max_segments */

> +       bool                    disable_bounce; /* disable bounce buffers */


Instead of adding a separate host configuration flag, let's use one of
capabilities bit-field instead (->caps or ->caps2). Then I suggest we
then add a new bit, MMC_CAP_NO_BOUNCE_BUFF.

>         unsigned short          unused;

>         unsigned int            max_req_size;   /* maximum number of bytes in one req */

>         unsigned int            max_blk_size;   /* maximum size of one mmc block */

> --

> 2.9.3

>


Kind regards
Uffe
--
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
Bartlomiej Zolnierkiewicz May 15, 2017, 2:04 p.m. UTC | #2
Hi,

[ I added Daniel, Marc & Steven to cc: ]

On Wednesday, May 10, 2017 10:24:14 AM Linus Walleij wrote:
> This option is activated by all multiplatform configs and what

> not so we almost always have it turned on, and the memory it

> saves is negligible, even more so moving forward. The actual

> bounce buffer only gets allocated only when used, the only

> thing the ifdefs are saving is a little bit of code.

> 

> It is highly improper to have this as a Kconfig option that

> get turned on by Kconfig, make this a pure runtime-thing and

> let the host decide whether we use bounce buffers. We add a

> new property "disable_bounce" to the host struct.

> 

> Notice that mmc_queue_calc_bouncesz() already disables the

> bounce buffers if host->max_segs != 1, so any arch that has a

> maximum number of segments higher than 1 will have bounce

> buffers disabled.

> 

> The option CONFIG_MMC_BLOCK_BOUNCE is default y so the

> majority of platforms in the kernel already have it on, and

> it then gets turned off at runtime since most of these have

> a host->max_segs > 1. The few exceptions that have

> host->max_segs == 1 and still turn off the bounce buffering

> are those that disable it in their defconfig.

> 

> Those are the following:

> 

> arch/arm/configs/colibri_pxa300_defconfig

> arch/arm/configs/zeus_defconfig

> - Uses MMC_PXA, drivers/mmc/host/pxamci.c

> - Sets host->max_segs = NR_SG, which is 1

> - This needs its bounce buffer deactivated so we set

>   host->disable_bounce to true in the host driver


[...]

> arch/mips/configs/cavium_octeon_defconfig

> - Uses MMC_CAVIUM_OCTEON, drivers/mmc/host/cavium.c

> - Sets host->max_segs to 16 or 1

> - Setting host->disable_bounce to be sure for the 1 case


From looking at the code it seems that bounce buffering should
be always beneficial to MMC performance when host->max_segs == 1.

It would be useful to know why these specific defconfigs
disable bounce buffering (to save memory?). Maybe the defaults
should be changed nowadays?

[...]

>  drivers/mmc/core/Kconfig  | 18 ------------------

>  drivers/mmc/core/queue.c  | 15 +--------------

>  drivers/mmc/host/cavium.c |  3 +++

>  drivers/mmc/host/pxamci.c |  6 ++++++

>  include/linux/mmc/host.h  |  1 +

>  5 files changed, 11 insertions(+), 32 deletions(-)

> 

> diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig

> index fc1ecdaaa9ca..42e89060cd41 100644

> --- a/drivers/mmc/core/Kconfig

> +++ b/drivers/mmc/core/Kconfig

> @@ -61,24 +61,6 @@ config MMC_BLOCK_MINORS

>  

>  	  If unsure, say 8 here.

>  

> -config MMC_BLOCK_BOUNCE

> -	bool "Use bounce buffer for simple hosts"

> -	depends on MMC_BLOCK

> -	default y

> -	help

> -	  SD/MMC is a high latency protocol where it is crucial to

> -	  send large requests in order to get high performance. Many

> -	  controllers, however, are restricted to continuous memory

> -	  (i.e. they can't do scatter-gather), something the kernel

> -	  rarely can provide.

> -

> -	  Say Y here to help these restricted hosts by bouncing

> -	  requests back and forth from a large buffer. You will get

> -	  a big performance gain at the cost of up to 64 KiB of

> -	  physical memory.

> -

> -	  If unsure, say Y here.

> -

>  config SDIO_UART

>  	tristate "SDIO UART/GPS class support"

>  	depends on TTY

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

> index 5c37b6be3e7b..545466342fb1 100644

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

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

> @@ -219,7 +219,6 @@ static struct mmc_queue_req *mmc_queue_alloc_mqrqs(int qdepth)

>  	return mqrq;

>  }

>  

> -#ifdef CONFIG_MMC_BLOCK_BOUNCE

>  static int mmc_queue_alloc_bounce_bufs(struct mmc_queue_req *mqrq, int qdepth,

>  				       unsigned int bouncesz)

>  {

> @@ -258,7 +257,7 @@ static unsigned int mmc_queue_calc_bouncesz(struct mmc_host *host)

>  {

>  	unsigned int bouncesz = MMC_QUEUE_BOUNCESZ;

>  

> -	if (host->max_segs != 1)

> +	if (host->max_segs != 1 || host->disable_bounce)

>  		return 0;

>  

>  	if (bouncesz > host->max_req_size)

> @@ -273,18 +272,6 @@ static unsigned int mmc_queue_calc_bouncesz(struct mmc_host *host)

>  

>  	return bouncesz;

>  }

> -#else

> -static inline bool mmc_queue_alloc_bounce(struct mmc_queue_req *mqrq,

> -					  int qdepth, unsigned int bouncesz)

> -{

> -	return false;

> -}

> -

> -static unsigned int mmc_queue_calc_bouncesz(struct mmc_host *host)

> -{

> -	return 0;

> -}

> -#endif

>  

>  static int mmc_queue_alloc_sgs(struct mmc_queue_req *mqrq, int qdepth,

>  			       int max_segs)

> diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c

> index 58b51ba6aabd..66066f73e477 100644

> --- a/drivers/mmc/host/cavium.c

> +++ b/drivers/mmc/host/cavium.c

> @@ -1050,6 +1050,9 @@ int cvm_mmc_of_slot_probe(struct device *dev, struct cvm_mmc_host *host)

>  	else

>  		mmc->max_segs = 1;

>  

> +	/* Disable bounce buffers for max_segs = 1 */

> +	mmc->disable_bounce = true;

> +

>  	/* DMA size field can address up to 8 MB */

>  	mmc->max_seg_size = 8 * 1024 * 1024;

>  	mmc->max_req_size = mmc->max_seg_size;

> diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c

> index c763b404510f..d3b5e6376504 100644

> --- a/drivers/mmc/host/pxamci.c

> +++ b/drivers/mmc/host/pxamci.c

> @@ -666,6 +666,12 @@ static int pxamci_probe(struct platform_device *pdev)

>  	mmc->max_segs = NR_SG;

>  

>  	/*

> +	 * This architecture used to disable bounce buffers through its

> +	 * defconfig, now it is done at runtime as a host property.

> +	 */

> +	mmc->disable_bounce = true;

> +

> +	/*

>  	 * Our hardware DMA can handle a maximum of one page per SG entry.

>  	 */

>  	mmc->max_seg_size = PAGE_SIZE;

> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h

> index 21385ac0c9b1..b53c0e18a33b 100644

> --- a/include/linux/mmc/host.h

> +++ b/include/linux/mmc/host.h

> @@ -310,6 +310,7 @@ struct mmc_host {

>  	/* host specific block data */

>  	unsigned int		max_seg_size;	/* see blk_queue_max_segment_size */

>  	unsigned short		max_segs;	/* see blk_queue_max_segments */

> +	bool			disable_bounce; /* disable bounce buffers */

>  	unsigned short		unused;

>  	unsigned int		max_req_size;	/* maximum number of bytes in one req */

>  	unsigned int		max_blk_size;	/* maximum size of one mmc block */


Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
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 May 18, 2017, 7:48 a.m. UTC | #3
On Mon, May 15, 2017 at 4:04 PM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:

> [ I added Daniel, Marc & Steven to cc: ]


>> The option CONFIG_MMC_BLOCK_BOUNCE is default y so the

>> majority of platforms in the kernel already have it on, and

>> it then gets turned off at runtime since most of these have

>> a host->max_segs > 1. The few exceptions that have

>> host->max_segs == 1 and still turn off the bounce buffering

>> are those that disable it in their defconfig.

>>

>> Those are the following:

>>

>> arch/arm/configs/colibri_pxa300_defconfig

>> arch/arm/configs/zeus_defconfig

>> - Uses MMC_PXA, drivers/mmc/host/pxamci.c

>> - Sets host->max_segs = NR_SG, which is 1

>> - This needs its bounce buffer deactivated so we set

>>   host->disable_bounce to true in the host driver

>

> [...]

>

>> arch/mips/configs/cavium_octeon_defconfig

>> - Uses MMC_CAVIUM_OCTEON, drivers/mmc/host/cavium.c

>> - Sets host->max_segs to 16 or 1

>> - Setting host->disable_bounce to be sure for the 1 case

>

> From looking at the code it seems that bounce buffering should

> be always beneficial to MMC performance when host->max_segs == 1.

>

> It would be useful to know why these specific defconfigs

> disable bounce buffering (to save memory?). Maybe the defaults

> should be changed nowadays?


I agree. But I can't test on pxamci and cavium so I would
appreciate if the driver maintainers try to remove the
flag (MMC_CAP_NO_BOUNCE_BUFF in the v2 patch set)
and see what happens.

I would be happy to cut this special flag altogether, but I am
also afraid of screwing up some config :/

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
Linus Walleij May 23, 2017, 9:05 a.m. UTC | #4
On Fri, May 19, 2017 at 9:30 AM, Steven J. Hill <Steven.Hill@cavium.com> wrote:

> Remove MMC bounce buffer config option and associated code. This

> is proposed in addition to Linus' changes to remove the config

> option. I have tested this on our Octeon hardware platforms.

>

> Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com>


This would have to be rebased as Ulf merged my patch making this
a per-host runtime config option. (The Kconfig is gone for example.)

Bounce buffers were added by Pierre Ossman for kernel 2.6.23 in
commit 98ccf14909ba02a41c5925b0b2c92aeeef23d3b9
"mmc: bounce requests for simple hosts"

Quote:

    Some hosts cannot do scatter/gather in hardware. Since not doing sg
    is such a big performance hit, we (optionally) bounce the requests
    to a simple linear buffer that we hand over to the driver.

    Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>


So this runs the risk on reducing performance on simple MMC/SD
controllers. Notice: simple, not old.

We need to know if people are deploying simple controllers still
and if this is something that really affects their performance.

That said: this was put in place because the kernel was sending
SG lists that the host DMA could not manage.

Nowadays we have two mechanisms:

- DMA engine and DMA-API that help out in managing bounce
  buffers when used. This means this only is useful for hardware
  that does autonomous DMA, without any separate DMA engine.

- CMA that can actually allocate a big chunk of memory: I think
  this original code is restricted to a 64KB segment because
  kmalloc() will only guarantee contigous physical memory up to
  64-128KiB or so. Now we could actually allocate a big badass
  CMA buffer if that improves the performance, and that would be
  a per-host setting.

It would be good to hear from people seeing benefits from bounce
buffers about this. What hardware is there that acually sees a
significant improvement with bounce buffers?

Pierre, what host were you developing this for? Maybe I can try
to get the same and test it.

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
Arnd Bergmann May 23, 2017, 10:08 a.m. UTC | #5
On Tue, May 23, 2017 at 11:05 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Fri, May 19, 2017 at 9:30 AM, Steven J. Hill <Steven.Hill@cavium.com> wrote:

>

>> Remove MMC bounce buffer config option and associated code. This

>> is proposed in addition to Linus' changes to remove the config

>> option. I have tested this on our Octeon hardware platforms.

>>

>> Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com>

>

> This would have to be rebased as Ulf merged my patch making this

> a per-host runtime config option. (The Kconfig is gone for example.)

>

> Bounce buffers were added by Pierre Ossman for kernel 2.6.23 in

> commit 98ccf14909ba02a41c5925b0b2c92aeeef23d3b9

> "mmc: bounce requests for simple hosts"

>

> Quote:

>

>     Some hosts cannot do scatter/gather in hardware. Since not doing sg

>     is such a big performance hit, we (optionally) bounce the requests

>     to a simple linear buffer that we hand over to the driver.

>

>     Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>

>

> So this runs the risk on reducing performance on simple MMC/SD

> controllers. Notice: simple, not old.


Right, we definitely need to have something that allows us
to send larger commands to the device for scattered requests.

It wouldn't hurt to reconsider whether the current method is
the most efficient way of doing this, but that doesn't seem to
be a priority to me. If the MMC bounce buffers get in the way
of the blk-mq conversion, we could try to come up with a
different implementation as part of that conversion.

> We need to know if people are deploying simple controllers still

> and if this is something that really affects their performance.

>

> That said: this was put in place because the kernel was sending

> SG lists that the host DMA could not manage.

>

> Nowadays we have two mechanisms:

>

> - DMA engine and DMA-API that help out in managing bounce

>   buffers when used. This means this only is useful for hardware

>   that does autonomous DMA, without any separate DMA engine.


I think the bounce buffers in the DMA-API (swiotlb) should be
kept separate since it has a completely different purpose of
bouncing pages within the dma mask, rather than linearizing
a sglist.

If I read this right, we used to use blk-bounce for highmem
pages and mmc-bounce for lowmem until 2ff1fa679115 ("mmc_block:
bounce buffer highmem support") in 2008, now we don't
use blk-bounce at all if mmc-bounce is in use.

> - CMA that can actually allocate a big chunk of memory: I think

>   this original code is restricted to a 64KB segment because

>   kmalloc() will only guarantee contigous physical memory up to

>   64-128KiB or so. Now we could actually allocate a big badass

>   CMA buffer if that improves the performance, and that would be

>   a per-host setting.

>

> It would be good to hear from people seeing benefits from bounce

> buffers about this. What hardware is there that acually sees a

> significant improvement with bounce buffers?


The mmc-bounce code is active on hosts that have 'max_segs=1'
(the default unless they override it). These one set it to '1'
explicitly:

drivers/mmc/host/au1xmmc.c:     mmc->max_segs = AU1XMMC_DESCRIPTOR_COUNT;
drivers/mmc/host/bfin_sdh.c:    mmc->max_segs = 1; /* only BF51x */
drivers/mmc/host/sdhci.c:               mmc->max_segs = 1; /* with
SDHCI_USE_SDMA */
drivers/mmc/host/ushc.c:        mmc->max_segs      = 1;
drivers/mmc/host/via-sdmmc.c:   mmc->max_segs = 1;

These have max_segs=1 but ask for bounce buffers to be
disabled, which will severely limit their performance:

drivers/mmc/host/cavium.c:              mmc->max_segs = 1; /*
cavium,octeon-6130-mmc */
drivers/mmc/host/pxamci.c: mmc->max_segs = NR_SG;

These ones don't seem to set it at all, defaulting to '1':
drivers/mmc/host/cb710-mmc.c
drivers/mmc/host/moxart-mmc.c
drivers/mmc/host/sdricoh_cs.c
drivers/mmc/host/toshsd.c

       Arnd
--
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
Pierre Ossman May 23, 2017, 6:24 p.m. UTC | #6
On 23/05/17 11:05, Linus Walleij wrote:
> 

> Pierre, what host were you developing this for? Maybe I can try

> to get the same and test it.

> 


I think the work primarily was done with the Texas Instruments SDHCI 
host that HP used in their laptops. Ricoh controllers were also popular, 
but they had some proprietary mode that you had to get them out of.

If I recall correctly, all early SDHCI controllers needed this feature. 
I think scatter/gather was a later addition to the spec.

I take it updates to the SD specification hasn't fixed this performance 
issue?

Regards
-- 
Pierre Ossman
--
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
diff mbox series

Patch

diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
index fc1ecdaaa9ca..42e89060cd41 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/mmc/core/Kconfig
@@ -61,24 +61,6 @@  config MMC_BLOCK_MINORS
 
 	  If unsure, say 8 here.
 
-config MMC_BLOCK_BOUNCE
-	bool "Use bounce buffer for simple hosts"
-	depends on MMC_BLOCK
-	default y
-	help
-	  SD/MMC is a high latency protocol where it is crucial to
-	  send large requests in order to get high performance. Many
-	  controllers, however, are restricted to continuous memory
-	  (i.e. they can't do scatter-gather), something the kernel
-	  rarely can provide.
-
-	  Say Y here to help these restricted hosts by bouncing
-	  requests back and forth from a large buffer. You will get
-	  a big performance gain at the cost of up to 64 KiB of
-	  physical memory.
-
-	  If unsure, say Y here.
-
 config SDIO_UART
 	tristate "SDIO UART/GPS class support"
 	depends on TTY
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 5c37b6be3e7b..545466342fb1 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -219,7 +219,6 @@  static struct mmc_queue_req *mmc_queue_alloc_mqrqs(int qdepth)
 	return mqrq;
 }
 
-#ifdef CONFIG_MMC_BLOCK_BOUNCE
 static int mmc_queue_alloc_bounce_bufs(struct mmc_queue_req *mqrq, int qdepth,
 				       unsigned int bouncesz)
 {
@@ -258,7 +257,7 @@  static unsigned int mmc_queue_calc_bouncesz(struct mmc_host *host)
 {
 	unsigned int bouncesz = MMC_QUEUE_BOUNCESZ;
 
-	if (host->max_segs != 1)
+	if (host->max_segs != 1 || host->disable_bounce)
 		return 0;
 
 	if (bouncesz > host->max_req_size)
@@ -273,18 +272,6 @@  static unsigned int mmc_queue_calc_bouncesz(struct mmc_host *host)
 
 	return bouncesz;
 }
-#else
-static inline bool mmc_queue_alloc_bounce(struct mmc_queue_req *mqrq,
-					  int qdepth, unsigned int bouncesz)
-{
-	return false;
-}
-
-static unsigned int mmc_queue_calc_bouncesz(struct mmc_host *host)
-{
-	return 0;
-}
-#endif
 
 static int mmc_queue_alloc_sgs(struct mmc_queue_req *mqrq, int qdepth,
 			       int max_segs)
diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c
index 58b51ba6aabd..66066f73e477 100644
--- a/drivers/mmc/host/cavium.c
+++ b/drivers/mmc/host/cavium.c
@@ -1050,6 +1050,9 @@  int cvm_mmc_of_slot_probe(struct device *dev, struct cvm_mmc_host *host)
 	else
 		mmc->max_segs = 1;
 
+	/* Disable bounce buffers for max_segs = 1 */
+	mmc->disable_bounce = true;
+
 	/* DMA size field can address up to 8 MB */
 	mmc->max_seg_size = 8 * 1024 * 1024;
 	mmc->max_req_size = mmc->max_seg_size;
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index c763b404510f..d3b5e6376504 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -666,6 +666,12 @@  static int pxamci_probe(struct platform_device *pdev)
 	mmc->max_segs = NR_SG;
 
 	/*
+	 * This architecture used to disable bounce buffers through its
+	 * defconfig, now it is done at runtime as a host property.
+	 */
+	mmc->disable_bounce = true;
+
+	/*
 	 * Our hardware DMA can handle a maximum of one page per SG entry.
 	 */
 	mmc->max_seg_size = PAGE_SIZE;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 21385ac0c9b1..b53c0e18a33b 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -310,6 +310,7 @@  struct mmc_host {
 	/* host specific block data */
 	unsigned int		max_seg_size;	/* see blk_queue_max_segment_size */
 	unsigned short		max_segs;	/* see blk_queue_max_segments */
+	bool			disable_bounce; /* disable bounce buffers */
 	unsigned short		unused;
 	unsigned int		max_req_size;	/* maximum number of bytes in one req */
 	unsigned int		max_blk_size;	/* maximum size of one mmc block */