diff mbox series

(Octeon) MMC performance degradation due to too many requests

Message ID ZM+IlctTTQLs7Qg9@lenoch
State New
Headers show
Series (Octeon) MMC performance degradation due to too many requests | expand

Commit Message

Ladislav Michl Aug. 6, 2023, 11:48 a.m. UTC
Hi Linus,

let me appologize for walking into dark past :) Here's some more context
for a start:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1691602.html
(note that "OK phew then I'm safe" ;-))

After moving a few Octeon 70XX based boards from vendor based 4.9 kernel
to 6.1 mainline a huge drop of MMC performance was observed.

While running badblocks on 2G partition takes 1min (±5s) and about 35k
interrupts on 4.9, recent kernels need over 3mins and 500k+ interrupts.

How do we get there?
ff4143ccff31 ("MIPS: Octeon: cavium_octeon_defconfig: Enable Octeon MMC")
enabled MMC driver, but left MMC_BLOCK_BOUNCE disabled, although driver
performace depends on it.
c3dccb74be28 ("mmc: core: Delete bounce buffer Kconfig option")
Added MMC_CAP_NO_BOUNCE_BUFF to the caps, based on assumption it should
be there as MMC_BLOCK_BOUNCE is disabled in defconfig
de3ee99b097d ("mmc: Delete bounce buffer handling")
finally removed all bounce buffer handling as almost nothing needs that.

Sadly, 70XX SoC cannot do SG, so it suffers a lot. Strangely enough,
above patches are either authored or suggested by Cavium's employees.

So, given the number of affected SoC and before cooking driver specific
solution, are we sure we indeed do not want some generic one?

While there, you might consider following patch:
-- >8 --
From: Ladislav Michl <ladis@linux-mips.org>
Subject: [PATCH] mmc: cavium: Remove misleading comment

Comment about disabling bounce buffers was added with c3dccb74be28
("mmc: core: Delete bounce buffer Kconfig option") and should be
deleted with de3ee99b097d ("mmc: Delete bounce buffer handling").
For the record, this driver should have never been used
MMC_CAP_NO_BOUNCE_BUFF as it hits performance badly for non SG
capable SoC.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
Fixes: de3ee99b097d ("mmc: Delete bounce buffer handling")
---
 drivers/mmc/host/cavium.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Linus Walleij Aug. 6, 2023, 8:44 p.m. UTC | #1
On Sun, Aug 6, 2023 at 1:48 PM Ladislav Michl <oss-lists@triops.cz> wrote:

> How do we get there?
> ff4143ccff31 ("MIPS: Octeon: cavium_octeon_defconfig: Enable Octeon MMC")
> enabled MMC driver, but left MMC_BLOCK_BOUNCE disabled, although driver
> performace depends on it.

Ooops.

> c3dccb74be28 ("mmc: core: Delete bounce buffer Kconfig option")
> Added MMC_CAP_NO_BOUNCE_BUFF to the caps, based on assumption it should
> be there as MMC_BLOCK_BOUNCE is disabled in defconfig
> de3ee99b097d ("mmc: Delete bounce buffer handling")
> finally removed all bounce buffer handling as almost nothing needs that.
>
> Sadly, 70XX SoC cannot do SG, so it suffers a lot. Strangely enough,
> above patches are either authored or suggested by Cavium's employees.
>
> So, given the number of affected SoC and before cooking driver specific
> solution, are we sure we indeed do not want some generic one?

So you are talking about something along the lines of:

commit bd9b902798ab14d19ca116b10bde581ddff8f905
Author: Linus Walleij <linus.walleij@linaro.org>
Date:   Mon Jan 29 00:44:53 2018 +0100

    mmc: sdhci: Implement an SDHCI-specific bounce buffer

?

Yeah I guess that if this is needed by more than one driver it
should be made into a library, or say a piece of code turned on by
a config option that the dependent drivers select.

Interested in the job? :D

Yours,
Linus Walleij
Ladislav Michl Aug. 6, 2023, 9:03 p.m. UTC | #2
On Sun, Aug 06, 2023 at 10:44:21PM +0200, Linus Walleij wrote:
> On Sun, Aug 6, 2023 at 1:48 PM Ladislav Michl <oss-lists@triops.cz> wrote:
> 
> > How do we get there?
> > ff4143ccff31 ("MIPS: Octeon: cavium_octeon_defconfig: Enable Octeon MMC")
> > enabled MMC driver, but left MMC_BLOCK_BOUNCE disabled, although driver
> > performace depends on it.
> 
> Ooops.
> 
> > c3dccb74be28 ("mmc: core: Delete bounce buffer Kconfig option")
> > Added MMC_CAP_NO_BOUNCE_BUFF to the caps, based on assumption it should
> > be there as MMC_BLOCK_BOUNCE is disabled in defconfig
> > de3ee99b097d ("mmc: Delete bounce buffer handling")
> > finally removed all bounce buffer handling as almost nothing needs that.
> >
> > Sadly, 70XX SoC cannot do SG, so it suffers a lot. Strangely enough,
> > above patches are either authored or suggested by Cavium's employees.
> >
> > So, given the number of affected SoC and before cooking driver specific
> > solution, are we sure we indeed do not want some generic one?
> 
> So you are talking about something along the lines of:
> 
> commit bd9b902798ab14d19ca116b10bde581ddff8f905
> Author: Linus Walleij <linus.walleij@linaro.org>
> Date:   Mon Jan 29 00:44:53 2018 +0100
> 
>     mmc: sdhci: Implement an SDHCI-specific bounce buffer
> 
> ?

Yes, this is the exact commit I had in mind :)

> Yeah I guess that if this is needed by more than one driver it
> should be made into a library, or say a piece of code turned on by
> a config option that the dependent drivers select.
> 
> Interested in the job? :D

Interested is not the word I'd use, but yes, I'll give it a try making it
a little more generic solution.

> Yours,
> Linus Walleij

Best reards,
	ladis
diff mbox series

Patch

diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c
index b58f003b10a4..71a9f91d2fc6 100644
--- a/drivers/mmc/host/cavium.c
+++ b/drivers/mmc/host/cavium.c
@@ -1032,8 +1032,6 @@  int cvm_mmc_of_slot_probe(struct device *dev, struct cvm_mmc_host *host)
 	 * We only have a 3.3v supply, we cannot support any
 	 * of the UHS modes. We do support the high speed DDR
 	 * modes up to 52MHz.
-	 *
-	 * Disable bounce buffers for max_segs = 1
 	 */
 	mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
 		     MMC_CAP_CMD23 | MMC_CAP_POWER_OFF_CARD | MMC_CAP_3_3V_DDR;