New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 721944 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 723759



Sign in to add a comment

Benchmark / pick SDHCI perf improvements?

Project Member Reported by diand...@chromium.org, May 12 2017

Issue description

In upstream, I noticed some SDHCI changes that are supposed to give a nice perf boost.  I believe they are all:

bd9ff18bffcd mmc: sdhci-pci: Do not set DMA mask in enable_dma()
741b48f4f397 mmc: sdhci-acpi: Remove enable_dma() hook
7b91369b4655 mmc: sdhci: Set DMA mask when adding host
fce1442164cf mmc: sdhci: further code simplication
df953925a504 mmc: sdhci: consolidate the DMA/ADMA size/address quicks
a0eaf0f93f06 mmc: sdhci: prepare DMA address/size quirk handling consolidation
add8913d5d73 mmc: sdhci: cleanup DMA un-mapping
94538e51d67d mmc: sdhci: clean up host cookie handling
f48f039cd2d3 mmc: sdhci: always unmap a mapped data transfer in sdhci_post_req()
c0999b720c4f mmc: sdhci: pass the cookie into sdhci_pre_dma_transfer()
60c647624a67 mmc: sdhci: factor out sdhci_pre_dma_transfer() from sdhci_adma_table_pre()
48857d9b7865 mmc: sdhci: move sdhci_pre_dma_transfer()
f55c98f7466c mmc: sdhci: factor out common DMA cleanup in sdhci_finish_data()
47fa961340d2 mmc: sdhci: avoid walking SG list for writes
acc3ad13832a mmc: sdhci: clean up coding style in sdhci_adma_table_pre()
e66e61cba1ff mmc: sdhci: allocate alignment and DMA descriptor buffer together
771a3dc22581 mmc: sdhci: further fix for DMA unmapping in sdhci_post_req()
054cedff5e02 mmc: sdhci: plug DMA mapping leak on error
edd63fcc97cd mmc: sdhci: avoid unnecessary mapping/unmapping of align buffer
2e4456f08fa8 mmc: sdhci: Fix strings broken across multiple lines


Those all picked cleanly to kernel 4.4, but then I need to find a good way to test their performance.  Anyone have a good way?

One of the patches, for instance, says:

>    Unnecessarily mapping and unmapping the align buffer for SD cards is
>    expensive: performance measurements on iMX6 show that this gives a hit
>    of 10% on hdparm buffered disk reads.

 
Owner: diand...@chromium.org
Status: Started (was: Available)
Gwendal suggests "hardware_StorageFio".  I'll give that a shot and see if I see anything.
Here's the runs both without and with patches.  Note that this was on a kernel w/ various lock / memory debugging, so we might be slower than a normal kernel.  ...but both runs were with the same extra options, so the two runs should be able to be compared to each other.


170516-emmctest_withoutpatches2.txt
88.2 KB View Download
170516-emmctest_withpatches3.txt
88.1 KB View Download
There's a ton of numbers, I picked a few bandwidth numbers out with the "withoutpatches" on the first line and "withpatches" on the second line

tl;dr it looks like some small improvements on most things with more noticable improvements on the "stress" read and write cases

/hardware_StorageFio _16k_read_read_bw 50432 
/hardware_StorageFio__16k_read_read_bw 51254

/hardware_StorageFio _16k_write_write_bw 43223
/hardware_StorageFio _16k_write_write_bw 43259

/hardware_StorageFio _seq_read_read_bw 86659
/hardware_StorageFio _seq_read_read_bw 86554

/hardware_StorageFio _seq_write_write_bw 39984
/hardware_StorageFio _seq_write_write_bw 38556


/hardware_StorageFio _1m_stress_read_bw 33585
/hardware_StorageFio _1m_stress_read_bw 37046

/hardware_StorageFio _1m_stress_write_bw 34205
/hardware_StorageFio _1m_stress_write_bw 38732


/hardware_StorageFio _boot_read_bw 38243
/hardware_StorageFio _boot_read_bw 39049

/hardware_StorageFio _login_read_bw 45621
/hardware_StorageFio _login_read_bw 45767

/hardware_StorageFio _surfing_read_bw 3277
/hardware_StorageFio _surfing_read_bw 3426

/hardware_StorageFio _surfing_write_bw 25249
/hardware_StorageFio _surfing_write_bw 26372

OK, thanks.  I actually ran the tests a few more times because I wasn't sure how noisy they were.  ...but I wasn't sure I should keep the numbers since I think I accidentally hit tried to interact with the device while the test was running (the ones I attached I definitely didn't touch)

...but in case it's helpful:

170516-emmctest_withoutpatches2.txt:/hardware_StorageFio   _1m_stress_write_bw                                                           34205
170516-emmctest_withoutpatches.txt:/hardware_StorageFio   _1m_stress_write_bw                                                           38896

170516-emmctest_withpatches2.txt:/hardware_StorageFio   _1m_stress_write_bw                                                           41252
170516-emmctest_withpatches3.txt:/hardware_StorageFio   _1m_stress_write_bw                                                           38732
170516-emmctest_withpatches.txt:/hardware_StorageFio   _1m_stress_write_bw                                                           38351

---

170516-emmctest_withoutpatches2.txt:/hardware_StorageFio   _1m_stress_read_bw                                                            33585
170516-emmctest_withoutpatches.txt:/hardware_StorageFio   _1m_stress_read_bw                                                            37229

170516-emmctest_withpatches2.txt:/hardware_StorageFio   _1m_stress_read_bw                                                            39690
170516-emmctest_withpatches3.txt:/hardware_StorageFio   _1m_stress_read_bw                                                            37046
170516-emmctest_withpatches.txt:/hardware_StorageFio   _1m_stress_read_bw                                                            37533

===

It looks like the numbers are quite noisy but there's a general trend higher.

===

Is this enough to actually try to land those patches, or I abandon them?
My opinion is yes
sorry - should have been more clear -- yes try to land them :-)
@5: oakie dokey.  I'll give them a quick once over and make sure that they don't look too stupid / broken / missing important stuff, then we can try to land them.

Since we're using sdhci on 3.18 products too, I wonder if we should consider the backport there, too?
re #7 -- yes if possible -- since we're pretty much all eMMC these days it'll help everybody, thanks!
Cc: groeck@chromium.org
I looked for mentions of regressions with the above patches and found one additional one that it would be good to take:

fc26fe9c3869 ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

---

OK, I did these things:
* Searched through linuxnext commits for mentions of the git hashes to find regression claims.  Only mention found was the one above.
* Skimmed commits landed "near" these commits.  Nothing else looked related.
* Re-checked again changes to sdhci (drivers/mmc/host/sdhci*) between our kernel and fc26fe9c3869 (the highest commit here).  Nothing looked related.
* Skimmed over the patches and confirmed they look sane.  I didn't dig into all of the changes nor give them a full review.

...so I think we're good.  Not 100% of these things is actually needed and some of the fixes are more "optimize the code" type patches, but maybe still nice to get ourselves caught up?


remote:   https://chromium-review.googlesource.com/508007 UPSTREAM: mmc: sdhci: Fix strings broken across multiple lines        
remote:   https://chromium-review.googlesource.com/508008 UPSTREAM: mmc: sdhci: avoid unnecessary mapping/unmapping of align buffer        
remote:   https://chromium-review.googlesource.com/508009 UPSTREAM: mmc: sdhci: plug DMA mapping leak on error        
remote:   https://chromium-review.googlesource.com/508010 UPSTREAM: mmc: sdhci: further fix for DMA unmapping in sdhci_post_req()        
remote:   https://chromium-review.googlesource.com/508011 UPSTREAM: mmc: sdhci: allocate alignment and DMA descriptor buffer together        
remote:   https://chromium-review.googlesource.com/508012 UPSTREAM: mmc: sdhci: clean up coding style in sdhci_adma_table_pre()        
remote:   https://chromium-review.googlesource.com/508013 UPSTREAM: mmc: sdhci: avoid walking SG list for writes        
remote:   https://chromium-review.googlesource.com/508014 UPSTREAM: mmc: sdhci: factor out common DMA cleanup in sdhci_finish_data()        
remote:   https://chromium-review.googlesource.com/508015 UPSTREAM: mmc: sdhci: move sdhci_pre_dma_transfer()        
remote:   https://chromium-review.googlesource.com/508016 UPSTREAM: mmc: sdhci: factor out sdhci_pre_dma_transfer() from sdhci_adma_tab...        
remote:   https://chromium-review.googlesource.com/508017 UPSTREAM: mmc: sdhci: pass the cookie into sdhci_pre_dma_transfer()        
remote:   https://chromium-review.googlesource.com/508018 UPSTREAM: mmc: sdhci: always unmap a mapped data transfer in sdhci_post_req()        
remote:   https://chromium-review.googlesource.com/508019 UPSTREAM: mmc: sdhci: clean up host cookie handling        
remote:   https://chromium-review.googlesource.com/508020 UPSTREAM: mmc: sdhci: cleanup DMA un-mapping        
remote:   https://chromium-review.googlesource.com/508021 UPSTREAM: mmc: sdhci: prepare DMA address/size quirk handling consolidation        
remote:   https://chromium-review.googlesource.com/508022 UPSTREAM: mmc: sdhci: consolidate the DMA/ADMA size/address quicks        
remote:   https://chromium-review.googlesource.com/508023 UPSTREAM: mmc: sdhci: further code simplication        
remote:   https://chromium-review.googlesource.com/508024 UPSTREAM: mmc: sdhci: Set DMA mask when adding host        
remote:   https://chromium-review.googlesource.com/508025 UPSTREAM: mmc: sdhci-acpi: Remove enable_dma() hook        
remote:   https://chromium-review.googlesource.com/508026 UPSTREAM: mmc: sdhci-pci: Do not set DMA mask in enable_dma()        
remote:   https://chromium-review.googlesource.com/508027 UPSTREAM: ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask        

===

I wasn't planning to add reviewers to the CLs to avoid spamming people that weren't interested, but hopefully someone will grab them.

===

One other note is that some of the patches were tagged as "stable 4.5+", which I totally don't understand.  I think they all should be fine on 4.4.  Similar with a few other fixes I discovered while looking through the commits.  See  bug #723759 .
Blockedon: 723759
Oh, actually it seems like  bug #723759  might be good to fix before this one.  At least one of the commits does mention a DMA leak in the case of an error, so we should take it.

I can still keep the two chains separate to avoid the big rebase now...
Project Member

Comment 11 by bugdroid1@chromium.org, May 19 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d94d0c2a0429ae4fb600dfbe2d7662cbad1a27f8

commit d94d0c2a0429ae4fb600dfbe2d7662cbad1a27f8
Author: Marek Vasut <marex@denx.de>
Date: Fri May 19 01:06:23 2017

UPSTREAM: mmc: sdhci: Fix strings broken across multiple lines

This is a trivial patch which fixes printed strings split across two
or more lines in the source. I tried to grep for some error output*,
but I couldn't find it easily because it was broken across multiple
lines. This patch makes my life easier.

* in particular "Timeout waiting for hardware interrupt."

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: Ie9f769c1529b7232c3dbdc0c123b9973d913132b
Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 2e4456f08fa81b9a4804379c56c7ef02c0b0d8f0)
Reviewed-on: https://chromium-review.googlesource.com/508007
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/d94d0c2a0429ae4fb600dfbe2d7662cbad1a27f8/drivers/mmc/host/sdhci.c

Project Member

Comment 12 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/618e4ed594e4f90b1259f3499dc517a8a0c5606f

commit 618e4ed594e4f90b1259f3499dc517a8a0c5606f
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:24 2017

UPSTREAM: mmc: sdhci: avoid unnecessary mapping/unmapping of align buffer

Unnecessarily mapping and unmapping the align buffer for SD cards is
expensive: performance measurements on iMX6 show that this gives a hit
of 10% on hdparm buffered disk reads.

MMC/SD card IO comes from the mm/vfs which gives us page based IO, so
for this case, the align buffer is not going to be used.  However, we
still map and unmap this buffer.

Eliminate this by switching the align buffer to be a DMA coherent
buffer, which needs no DMA maintenance to access the buffer.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: I25bb969044cf5ef4ced7f0f8b57b9bc3eccbb4b4
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.5+
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit edd63fcc97cdb53279a7c43fa1691f5913d92793)
Reviewed-on: https://chromium-review.googlesource.com/508008
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/618e4ed594e4f90b1259f3499dc517a8a0c5606f/drivers/mmc/host/sdhci.c

Project Member

Comment 13 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c8f2eeeeea56034fd82c09935052e5ea80e771a7

commit c8f2eeeeea56034fd82c09935052e5ea80e771a7
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:25 2017

UPSTREAM: mmc: sdhci: plug DMA mapping leak on error

If we terminate a command early, we fail to properly clean up the DMA
mappings for the data part of the request.  Put this clean up to the
tasklet, which is the common path for finishing a request so we always
clean up after ourselves.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: If3d2dd5aa3dbc55149f2386f78fa37db7b0014ca
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
[ Split original patch so that it now contains only the fix ]
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.5+
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 054cedff5e025a54ceefff891c6ea42ee8b37eab)
Reviewed-on: https://chromium-review.googlesource.com/508009
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/c8f2eeeeea56034fd82c09935052e5ea80e771a7/drivers/mmc/host/sdhci.c

Project Member

Comment 14 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/91ae7f0154721fe1b97f2a34c250a990845ff8fd

commit 91ae7f0154721fe1b97f2a34c250a990845ff8fd
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:26 2017

UPSTREAM: mmc: sdhci: further fix for DMA unmapping in sdhci_post_req()

sdhci_post_req() exists to unmap a previously mapped but already
finished request, while the next request is in progress.  However, the
state of the SDHCI_REQ_USE_DMA flag depends on the last submitted
request.

This means we can end up clearing the flag due to a quirk, which then
means that sdhci_post_req() fails to unmap the DMA buffer, potentially
leading to data corruption.

We can safely ignore the SDHCI_REQ_USE_DMA here, as testing
data->host_cookie is entirely sufficient.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: Iba50dcfc3dfc64c6e2ffee4421a51bfa72fe2968
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
[ Re-based to apply as a separate fix ]
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.5+
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 771a3dc225815b7cc691c1ce703a3af8488e48df)
Reviewed-on: https://chromium-review.googlesource.com/508010
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/91ae7f0154721fe1b97f2a34c250a990845ff8fd/drivers/mmc/host/sdhci.c

Project Member

Comment 15 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/07a1976f20eb1e9b19c7cdcc9ee414add4d9c811

commit 07a1976f20eb1e9b19c7cdcc9ee414add4d9c811
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:28 2017

UPSTREAM: mmc: sdhci: allocate alignment and DMA descriptor buffer together

Allocate both the alignment and DMA descriptor buffers together.  The
size of the alignment buffer will always be aligned to the hosts
required alignment, which gives appropriate alignment to the DMA
descriptors.

We have a maximum of 128 segments, and a maximum alignment of 64 bits.
This gives a maximum alignment buffer size of 1024 bytes.

The DMA descriptors are a maximum of 12 bytes, and we allocate 128 * 2
+ 1 of these, which gives a maximum DMA descriptor buffer size of 3084
bytes.

This means the allocation for a 4K page sized system will be an order-1
allocation, since the resulting overall size is 4108.  This is more
prone to failure than page-sized allocations, but since this allocation
commonly occurs at startup, the chances of failure are small.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: I005c6cf597bd8b7f77fa67066d9a7fb6e897fab2
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
[ Changed to check ADMA table alignment ]
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit e66e61cba1fffc53151b5303688f9c077f87104c)
Reviewed-on: https://chromium-review.googlesource.com/508011
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/07a1976f20eb1e9b19c7cdcc9ee414add4d9c811/drivers/mmc/host/sdhci.c

Project Member

Comment 16 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7cf956dfbdd3c7df106fd77200db2db4493b9235

commit 7cf956dfbdd3c7df106fd77200db2db4493b9235
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:29 2017

UPSTREAM: mmc: sdhci: clean up coding style in sdhci_adma_table_pre()

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: I4199e16055f7adfa9542864c5490476c9c84d3b5
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit acc3ad13832ab1aa01bdf9b7e1d76171d1e4d060)
Reviewed-on: https://chromium-review.googlesource.com/508012
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/7cf956dfbdd3c7df106fd77200db2db4493b9235/drivers/mmc/host/sdhci.c

Project Member

Comment 17 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/99cb4bb275eb6ebd343a1ed1bbb9d627b1fac590

commit 99cb4bb275eb6ebd343a1ed1bbb9d627b1fac590
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:30 2017

UPSTREAM: mmc: sdhci: avoid walking SG list for writes

If we are writing data to the card, there is no point in walking the
scatterlist to find out if there are any unaligned entries; this is a
needless waste of CPU cycles.  Avoid this by checking for a non-read
tranfer first.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: I123f34c69cbefd1c70f6cf324489a07290cb23a8
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 47fa961340d2181ebf1165b7651dcbb1b1029163)
Reviewed-on: https://chromium-review.googlesource.com/508013
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/99cb4bb275eb6ebd343a1ed1bbb9d627b1fac590/drivers/mmc/host/sdhci.c

Project Member

Comment 18 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2d1e82768949cdfeff9a6996f5a53aec8593d316

commit 2d1e82768949cdfeff9a6996f5a53aec8593d316
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:31 2017

UPSTREAM: mmc: sdhci: factor out common DMA cleanup in sdhci_finish_data()

sdhci_finish_data() has two paths which result in identical DMA cleanup.
One is when SDHCI_USE_ADMA is clear, and the other is just before when
SDHCI_USE_ADMA is set, and is performed within sdhci_adma_table_post().

Simplify the code by removing the 'else' and eliminating the duplicate
inside sdhci_adma_table_post().

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: I65099f68e63655e971fc8c0b95a1e5703f0bca30
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit f55c98f7466c2e52125d6ffd69295c0158ac609a)
Reviewed-on: https://chromium-review.googlesource.com/508014
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/2d1e82768949cdfeff9a6996f5a53aec8593d316/drivers/mmc/host/sdhci.c

Project Member

Comment 19 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f6b433fded66db9bda9fffb0296f69c04bafc245

commit f6b433fded66db9bda9fffb0296f69c04bafc245
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:32 2017

UPSTREAM: mmc: sdhci: move sdhci_pre_dma_transfer()

Move sdhci_pre_dma_transfer() to avoid needing to declare this function
before use.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: Id8282108983b2f4aa556ec8d089582e2556068b0
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 48857d9b7865c4110ecf9c57b85224a3ec84ad54)
Reviewed-on: https://chromium-review.googlesource.com/508015
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/f6b433fded66db9bda9fffb0296f69c04bafc245/drivers/mmc/host/sdhci.c

Project Member

Comment 20 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/aebb9a31a98c865ad12621d4bed21e97b5e1787f

commit aebb9a31a98c865ad12621d4bed21e97b5e1787f
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:37 2017

UPSTREAM: mmc: sdhci: clean up host cookie handling

Commit d31911b9374a ("mmc: sdhci: fix dma memory leak in sdhci_pre_req()")
added a complicated method to manage the DMA map state for the data
transfer, but this complexity is not required.

There are three states:
* Unmapped
* Mapped by sdhci_pre_req()
* Mapped by sdhci_prepare_data()

sdhci_prepare_data() needs to know when the data buffers have been
successfully mapped by sdhci_pre_req(), and if so, there is no need to
map them a second time.

When we come to tear down the mapping, we want to know whether
sdhci_post_req() will be called (which is determined by sdhci_pre_req()
having been previously called) so that we can postpone the unmap
operation.

Hence, it makes sense to simply record when the successful DMA map
happened (via COOKIE_PRE_MAPPED vs COOKIE_MAPPED) rather than having
the complex mechanics involving COOKIE_MAPPED vs COOKIE_GIVEN.

If a mapping is created by sdhci_prepare_data(), we must tear it down
ourselves, without waiting for sdhci_post_req() (hence, the new
COOKIE_MAPPED case).  If the mapping is created by sdhci_pre_req()
then sdhci_post_req() is responsible for tearing the mapping down.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: Id1a5fb479789782674dbb839b93bdc1874608720
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 94538e51d67da97e798d379d6bcf3d386d609bfb)
Reviewed-on: https://chromium-review.googlesource.com/508019
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/aebb9a31a98c865ad12621d4bed21e97b5e1787f/drivers/mmc/host/sdhci.c
[modify] https://crrev.com/aebb9a31a98c865ad12621d4bed21e97b5e1787f/drivers/mmc/host/sdhci.h

Project Member

Comment 21 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/dc724d61060a5550f008bea0813c772d6dc0ea2a

commit dc724d61060a5550f008bea0813c772d6dc0ea2a
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:38 2017

UPSTREAM: mmc: sdhci: cleanup DMA un-mapping

The patch "mmc: sdhci: plug DMA mapping leak on error" added
un-mapping logic to sdhci_tasklet_finish() where it is always
called, thereby preventing the mapping leaking.

Consequently the un-mapping code in sdhci_finish_data() is no
longer needed.  Remove it.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: Ic8261739e1edcbe6eff5d0fb57845403aa4f7857
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
[ Split from original "mmc: sdhci: plug DMA mapping leak on error" patch ]
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit add8913d5d73fe617fd7fa0f62f170c8ac5f313b)
Reviewed-on: https://chromium-review.googlesource.com/508020
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/dc724d61060a5550f008bea0813c772d6dc0ea2a/drivers/mmc/host/sdhci.c

Project Member

Comment 22 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/995dd0b06e908883a14af52118163f8e21deb173

commit 995dd0b06e908883a14af52118163f8e21deb173
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:39 2017

UPSTREAM: mmc: sdhci: prepare DMA address/size quirk handling consolidation

Prepare to consolidate the DMA address/size quirk handling into one
single loop.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: If73075cc0e76701644a760d1e9bde411d10efa8c
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit a0eaf0f93f0630ac09519e27c84f88c8e41c6f8b)
Reviewed-on: https://chromium-review.googlesource.com/508021
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/995dd0b06e908883a14af52118163f8e21deb173/drivers/mmc/host/sdhci.c

Project Member

Comment 23 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/15b72c1b94b7c9da2211e1741d411f68b345f6ba

commit 15b72c1b94b7c9da2211e1741d411f68b345f6ba
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:40 2017

UPSTREAM: mmc: sdhci: consolidate the DMA/ADMA size/address quicks

Rather than scanning the scatterlist multiple times for each quirk,
scan it once, checking for each possible quirk.  This should be
cheaper due to the length and offset members commonly sharing the
same cache line than scanning the scatterlist multiple times.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: If42b6c0b1f5d635b8f2fa652494180d14bf3d971
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit df953925a504d3c7a2a8814143a044c14d6660c0)
Reviewed-on: https://chromium-review.googlesource.com/508022
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/15b72c1b94b7c9da2211e1741d411f68b345f6ba/drivers/mmc/host/sdhci.c

Project Member

Comment 24 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c639e461f146e7ce2baacbf4ad87ecebaa24fa22

commit c639e461f146e7ce2baacbf4ad87ecebaa24fa22
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri May 19 01:06:41 2017

UPSTREAM: mmc: sdhci: further code simplication

Further simplify the code in sdhci_prepare_data() - we don't set
SDHCI_REQ_USE_DMA anywhere else in the driver, so there is no
need to set it, and then immediately test it.

BUG= chromium:721944 
TEST=hardware_StorageFio

Change-Id: Iab6886da4889297bbb807067eb37836a6f9e24f5
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit fce1442164cff51250c8f264b3757fd76f2a955b)
Reviewed-on: https://chromium-review.googlesource.com/508023
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/c639e461f146e7ce2baacbf4ad87ecebaa24fa22/drivers/mmc/host/sdhci.c

Status: Fixed (was: Started)
Not sure if this will make M-60, or M-61.
Labels: M-60
Oops, off my one week on the branch.  Def M-60.

Comment 27 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment