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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html