Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
Hi Simon, On 04/09/2016 10:55 AM, Simon Glass wrote: > On 20 March 2016 at 16:54, Eric Nelson wrote: >> On 03/20/2016 03:13 PM, Tom Rini wrote: >>> On Sun, Mar 20, 2016 at 12:35:53PM -0700, Eric Nelson wrote: On 03/17/2016 02:23 PM, Stephen Warren wrote: > On 03/16/2016 03:40 PM, Eric Nelson wrote: >> Signed-off-by: Eric Nelson > > Patch description. > >> --- >> drivers/mmc/mmc.c | 10 +- >> drivers/mmc/mmc_write.c | 7 +++ > > Presumably it makes sense for the cache to work for IDE, SATA, USB, > SCSI, ... too. I wonder if it's possible to put this code somewhere more > central than mmc*.c so it automatically applies to > dev_desc->block_read() (see include/part.h). Perhaps not since each > implementation supplies its own block_read function directly, so the > cache calls do need to be duplicated everywhere. > Yeah. I haven't found a spot that would allow interception of the various block_read/write functions. >>> >>> Shouldn't DM also help here? >> >> I haven't yet looked, but this may be true. >> >> I'm seeing some build breakage on master surrounding the use >> of DM though. >> >> If I select DM and BLK on top of nitrogen6q_defconfig, I get >> lots of build errors. >> >> I want to get a V2 RFC patch out before digging through the >> details of that. > > I'm about to send out a series that rationalises the block devices a > bit. In the meantime, see u-boot-dm/blkb-working for some MMC ideas. > I figured things out and sent V2 and V3 versions of these patches. Tom applied V3 to master, though I do still have three patches to address Stephen's review pending: http://lists.denx.de/pipermail/u-boot/2016-April/thread.html#250331 https://patchwork.ozlabs.org/patch/605421/ https://patchwork.ozlabs.org/patch/605420/ https://patchwork.ozlabs.org/patch/605422/ Regards, Eric ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
Hi Eric, On 20 March 2016 at 16:54, Eric Nelson wrote: > Hi Tom, > > On 03/20/2016 03:13 PM, Tom Rini wrote: >> On Sun, Mar 20, 2016 at 12:35:53PM -0700, Eric Nelson wrote: >>> Hi Stephen, >>> >>> On 03/17/2016 02:23 PM, Stephen Warren wrote: On 03/16/2016 03:40 PM, Eric Nelson wrote: > Signed-off-by: Eric Nelson Patch description. > --- > drivers/mmc/mmc.c | 10 +- > drivers/mmc/mmc_write.c | 7 +++ Presumably it makes sense for the cache to work for IDE, SATA, USB, SCSI, ... too. I wonder if it's possible to put this code somewhere more central than mmc*.c so it automatically applies to dev_desc->block_read() (see include/part.h). Perhaps not since each implementation supplies its own block_read function directly, so the cache calls do need to be duplicated everywhere. >>> >>> Yeah. I haven't found a spot that would allow interception of >>> the various block_read/write functions. >> >> Shouldn't DM also help here? >> > > I haven't yet looked, but this may be true. > > I'm seeing some build breakage on master surrounding the use > of DM though. > > If I select DM and BLK on top of nitrogen6q_defconfig, I get > lots of build errors. > > I want to get a V2 RFC patch out before digging through the > details of that. I'm about to send out a series that rationalises the block devices a bit. In the meantime, see u-boot-dm/blkb-working for some MMC ideas. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
Hi all, On 03/21/2016 11:31 AM, Eric Nelson wrote: > On 03/20/2016 03:54 PM, Eric Nelson wrote: >> On 03/20/2016 03:13 PM, Tom Rini wrote: >>> On Sun, Mar 20, 2016 at 12:35:53PM -0700, Eric Nelson wrote: On 03/17/2016 02:23 PM, Stephen Warren wrote: > On 03/16/2016 03:40 PM, Eric Nelson wrote: >> Signed-off-by: Eric Nelson > ... >> I'm seeing some build breakage on master surrounding the use >> of DM though. >> Peng's patch made it clear that DM wasn't supported by fsl_esdhc. >> If I select DM and BLK on top of nitrogen6q_defconfig, I get >> lots of build errors. >> It's CONFIG_BLK that produces lots of issues, and from what I can tell, it's only currently supported for sandbox. Out of ignorance, I was conflating the two. >> I want to get a V2 RFC patch out before digging through the >> details of that. >> > > I'm obviously not up to speed on the state of DM and I hadn't > seen Simon's patch adding blk.h. > > The new blk_dread/write/erase functions do provide a convenient > spot for checking cache, though they're not universally used yet. > > In particular, hooking up the cache there will lose visibility > into things like the "mmc write" command. > > I'm also not sure of the current state of DM with respect to > block drivers and wonder if a block cache should wait a cycle > or two. > > Simon, I'd appreciate some feedback when you have a chance. > I think I have a better handle on this now. I'm still a bit confused on what needs to be done in order for CONFIG_BLK to work against real hardware. >From what I can tell, the some modules in cmd/ need to be updated to use blk_dread/blk_dwrite/blk_derase and some kind of re-structuring needs to occur in drivers/mmc to support the "blk" uclass. Does that sound about right? Is somebody currently working on this? Please advise, Eric ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
Hi Tom, On 03/20/2016 03:54 PM, Eric Nelson wrote: > On 03/20/2016 03:13 PM, Tom Rini wrote: >> On Sun, Mar 20, 2016 at 12:35:53PM -0700, Eric Nelson wrote: >>> On 03/17/2016 02:23 PM, Stephen Warren wrote: On 03/16/2016 03:40 PM, Eric Nelson wrote: > Signed-off-by: Eric Nelson Patch description. > --- > drivers/mmc/mmc.c | 10 +- > drivers/mmc/mmc_write.c | 7 +++ Presumably it makes sense for the cache to work for IDE, SATA, USB, SCSI, ... too. I wonder if it's possible to put this code somewhere more central than mmc*.c so it automatically applies to dev_desc->block_read() (see include/part.h). Perhaps not since each implementation supplies its own block_read function directly, so the cache calls do need to be duplicated everywhere. >>> >>> Yeah. I haven't found a spot that would allow interception of >>> the various block_read/write functions. >> >> Shouldn't DM also help here? >> > > I haven't yet looked, but this may be true. > > I'm seeing some build breakage on master surrounding the use > of DM though. > > If I select DM and BLK on top of nitrogen6q_defconfig, I get > lots of build errors. > > I want to get a V2 RFC patch out before digging through the > details of that. > I'm obviously not up to speed on the state of DM and I hadn't seen Simon's patch adding blk.h. The new blk_dread/write/erase functions do provide a convenient spot for checking cache, though they're not universally used yet. In particular, hooking up the cache there will lose visibility into things like the "mmc write" command. I'm also not sure of the current state of DM with respect to block drivers and wonder if a block cache should wait a cycle or two. Simon, I'd appreciate some feedback when you have a chance. Regards, Eric ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
Hi Stephen, On 03/20/2016 12:35 PM, Eric Nelson wrote: > On 03/17/2016 02:23 PM, Stephen Warren wrote: >> On 03/16/2016 03:40 PM, Eric Nelson wrote: ... >> Do you have any stats on how many operations this saves for typical FS >> operations such as: >> >> - Partition table type identification (with various types such as >> MBR/DOS, GPT, ...) >> - Partition enumeration >> - Filesystem identification (with various filesystems such as FAT, ext, >> ...) >> - File reads >> > > Not yet, but I'm working something up that will allow this to be > gathered easily. As soon as we implement a cache, it provides a nice > spot for tracing operations. > The V2 RFC patch set implements a command (blkcache) that allows tracing block read operations as they pass through the cache, among other things: => blkcache trace => load mmc 0 10008000 /zImage miss: start 0, count 1 fill: start 0, count 1 miss: start 2000, count 1 fill: start 2000, count 1 reading /zImage hit: start 2000, count 1 miss: start 2011, count 2 miss: start 2001, count 6 miss: start 2031, count 9678 miss: start 45ff, count 1 fill: start 45ff, count 1 4955304 bytes read in 258 ms (18.3 MiB/s) http://lists.denx.de/pipermail/u-boot/2016-March/249155.html https://patchwork.ozlabs.org/patch/599942/ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
Hi Tom, On 03/20/2016 03:13 PM, Tom Rini wrote: > On Sun, Mar 20, 2016 at 12:35:53PM -0700, Eric Nelson wrote: >> Hi Stephen, >> >> On 03/17/2016 02:23 PM, Stephen Warren wrote: >>> On 03/16/2016 03:40 PM, Eric Nelson wrote: Signed-off-by: Eric Nelson >>> >>> Patch description. >>> --- drivers/mmc/mmc.c | 10 +- drivers/mmc/mmc_write.c | 7 +++ >>> >>> Presumably it makes sense for the cache to work for IDE, SATA, USB, >>> SCSI, ... too. I wonder if it's possible to put this code somewhere more >>> central than mmc*.c so it automatically applies to >>> dev_desc->block_read() (see include/part.h). Perhaps not since each >>> implementation supplies its own block_read function directly, so the >>> cache calls do need to be duplicated everywhere. >>> >> >> Yeah. I haven't found a spot that would allow interception of >> the various block_read/write functions. > > Shouldn't DM also help here? > I haven't yet looked, but this may be true. I'm seeing some build breakage on master surrounding the use of DM though. If I select DM and BLK on top of nitrogen6q_defconfig, I get lots of build errors. I want to get a V2 RFC patch out before digging through the details of that. Regards, Eric ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
On Sun, Mar 20, 2016 at 12:35:53PM -0700, Eric Nelson wrote: > Hi Stephen, > > On 03/17/2016 02:23 PM, Stephen Warren wrote: > > On 03/16/2016 03:40 PM, Eric Nelson wrote: > >> Signed-off-by: Eric Nelson > > > > Patch description. > > > >> --- > >> drivers/mmc/mmc.c | 10 +- > >> drivers/mmc/mmc_write.c | 7 +++ > > > > Presumably it makes sense for the cache to work for IDE, SATA, USB, > > SCSI, ... too. I wonder if it's possible to put this code somewhere more > > central than mmc*.c so it automatically applies to > > dev_desc->block_read() (see include/part.h). Perhaps not since each > > implementation supplies its own block_read function directly, so the > > cache calls do need to be duplicated everywhere. > > > > Yeah. I haven't found a spot that would allow interception of > the various block_read/write functions. Shouldn't DM also help here? -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
Hi Stephen, On 03/17/2016 02:23 PM, Stephen Warren wrote: > On 03/16/2016 03:40 PM, Eric Nelson wrote: >> Signed-off-by: Eric Nelson > > Patch description. > >> --- >> drivers/mmc/mmc.c | 10 +- >> drivers/mmc/mmc_write.c | 7 +++ > > Presumably it makes sense for the cache to work for IDE, SATA, USB, > SCSI, ... too. I wonder if it's possible to put this code somewhere more > central than mmc*.c so it automatically applies to > dev_desc->block_read() (see include/part.h). Perhaps not since each > implementation supplies its own block_read function directly, so the > cache calls do need to be duplicated everywhere. > Yeah. I haven't found a spot that would allow interception of the various block_read/write functions. The get_dev_hwpart() routine in disk/part.c is close, but it seems to be bypassed by cmd_mmc, cmd_sata, et cetera. I think the best that can be done is to provide a common shim that can easily be inserted from within the various block driver code. >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c >>* >>* SPDX-License-Identifier:GPL-2.0+ >>*/ >> - >> #include > > Nit: unrelated change. > > I think there is a missing call to cache_block_invalidate() when the MMC > device gets re-enumerated/re-initialized. The user would do something to > trigger this (e.g. mmc rescan) when they'd swapped an SD card out for > example. > Good catch. > Do you have any stats on how many operations this saves for typical FS > operations such as: > > - Partition table type identification (with various types such as > MBR/DOS, GPT, ...) > - Partition enumeration > - Filesystem identification (with various filesystems such as FAT, ext, > ...) > - File reads > Not yet, but I'm working something up that will allow this to be gathered easily. As soon as we implement a cache, it provides a nice spot for tracing operations. Regards, Eric ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
Signed-off-by: Eric Nelson
---
drivers/mmc/mmc.c | 10 +-
drivers/mmc/mmc_write.c | 7 +++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 8b2e606..956f4e1 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -6,7 +6,6 @@
*
* SPDX-License-Identifier:GPL-2.0+
*/
-
#include
#include
#include
@@ -240,6 +239,8 @@ static ulong mmc_bread(struct blk_desc *block_dev, lbaint_t
start,
int dev_num = block_dev->devnum;
int err;
lbaint_t cur, blocks_todo = blkcnt;
+ void *outbuf = dst;
+ lbaint_t outblk = start;
if (blkcnt == 0)
return 0;
@@ -260,6 +261,10 @@ static ulong mmc_bread(struct blk_desc *block_dev,
lbaint_t start,
return 0;
}
+ if (cache_block_read(IF_TYPE_MMC, dev_num, start, blkcnt,
+mmc->read_bl_len, dst))
+ return blkcnt;
+
if (mmc_set_blocklen(mmc, mmc->read_bl_len)) {
debug("%s: Failed to set blocklen\n", __func__);
return 0;
@@ -277,6 +282,9 @@ static ulong mmc_bread(struct blk_desc *block_dev, lbaint_t
start,
dst += cur * mmc->read_bl_len;
} while (blocks_todo > 0);
+ cache_block_fill(IF_TYPE_MMC, dev_num, outblk, blkcnt,
+mmc->read_bl_len, outbuf);
+
return blkcnt;
}
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
index 7b186f8..a877c78 100644
--- a/drivers/mmc/mmc_write.c
+++ b/drivers/mmc/mmc_write.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "mmc_private.h"
static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
@@ -20,6 +21,8 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start,
lbaint_t blkcnt)
ulong end;
int err, start_cmd, end_cmd;
+ cache_block_invalidate(IF_TYPE_MMC, mmc->block_dev.dev);
+
if (mmc->high_capacity) {
end = start + blkcnt - 1;
} else {
@@ -82,6 +85,8 @@ unsigned long mmc_berase(struct blk_desc *block_dev, lbaint_t
start,
if (err < 0)
return -1;
+ cache_block_invalidate(IF_TYPE_MMC, dev_num);
+
/*
* We want to see if the requested start or total block count are
* unaligned. We discard the whole numbers and only care about the
@@ -186,6 +191,8 @@ ulong mmc_bwrite(struct blk_desc *block_dev, lbaint_t
start, lbaint_t blkcnt,
if (err < 0)
return 0;
+ cache_block_invalidate(IF_TYPE_MMC, dev_num);
+
if (mmc_set_blocklen(mmc, mmc->write_bl_len))
return 0;
--
2.6.2
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 2/2] mmc: add support for block device cache
On 03/16/2016 03:40 PM, Eric Nelson wrote: Signed-off-by: Eric Nelson Patch description. --- drivers/mmc/mmc.c | 10 +- drivers/mmc/mmc_write.c | 7 +++ Presumably it makes sense for the cache to work for IDE, SATA, USB, SCSI, ... too. I wonder if it's possible to put this code somewhere more central than mmc*.c so it automatically applies to dev_desc->block_read() (see include/part.h). Perhaps not since each implementation supplies its own block_read function directly, so the cache calls do need to be duplicated everywhere. diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c * * SPDX-License-Identifier: GPL-2.0+ */ - #include Nit: unrelated change. I think there is a missing call to cache_block_invalidate() when the MMC device gets re-enumerated/re-initialized. The user would do something to trigger this (e.g. mmc rescan) when they'd swapped an SD card out for example. Do you have any stats on how many operations this saves for typical FS operations such as: - Partition table type identification (with various types such as MBR/DOS, GPT, ...) - Partition enumeration - Filesystem identification (with various filesystems such as FAT, ext, ...) - File reads ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
