Re: [U-Boot] [PATCH 1/3] drivers: block: add block device cache

[Eric Nelson]

Thanks Tom,

On 03/28/2016 07:16 AM, Tom Rini wrote:
> On Sun, Mar 27, 2016 at 12:00:13PM -0700, Eric Nelson wrote:
> 
>> +++ b/drivers/block/blkcache.c
> [snip]
>> +static int trace;
> 
> I see where you can toggle this at run-time.  But do we really think
> that this will be useful outside of debug?  My first reaction is that we
> should move the trace stuff into debug() statements instead.
> 

Will do.

Stephen had the same comment.

> [snip]
>> +#ifdef CONFIG_CMD_BLOCK_CACHE
> 
> Please split the command code into cmd/blkcache.c.  And yes, this might
> require thinking harder about what to expose or making an API for some
> of it.  I'm also not sure that's a bad thing as tuning the cache seems
> useful long term but dumping the stats seems more like debug work.
> 

Okay. I started to do that but stopped when I looked at the number
of implementation details needed by the commands themselves.

> [snip]
>> +/* Strip off leading 'i2c' command argument */
>> +argc--;
>> +argv++;
> 
> I see you borrowed from i2c here :)
> 
Yep. Oops.

Regards,


Eric
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] drivers: block: add block device cache

[Tom Rini]

On Sun, Mar 27, 2016 at 12:00:13PM -0700, Eric Nelson wrote:

> +++ b/drivers/block/blkcache.c
[snip]
> +static int trace;

I see where you can toggle this at run-time.  But do we really think
that this will be useful outside of debug?  My first reaction is that we
should move the trace stuff into debug() statements instead.

[snip]
> +#ifdef CONFIG_CMD_BLOCK_CACHE

Please split the command code into cmd/blkcache.c.  And yes, this might
require thinking harder about what to expose or making an API for some
of it.  I'm also not sure that's a bad thing as tuning the cache seems
useful long term but dumping the stats seems more like debug work.

[snip]
> + /* Strip off leading 'i2c' command argument */
> + argc--;
> + argv++;

I see you borrowed from i2c here :)

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/3] drivers: block: add block device cache

[Eric Nelson]

Add a block device cache to speed up repeated reads of block devices by 
various filesystems.

This small amount of cache can dramatically speed up filesystem
operations by skipping repeated reads of common areas of a block
device (typically directory structures).

This has shown to have some benefit on FAT filesystem operations of
loading a kernel and RAM disk, but more dramatic benefits on ext4
filesystems when the kernel and/or RAM disk are spread across 
multiple extent header structures as described in commit fc0fc50.

The cache is implemented through a minimal list (block_cache) maintained
in most-recently-used order and count of the current number of entries
(cache_count). It uses a maximum block count setting to prevent copies
of large block reads and an upper bound on the number of cached areas.

The maximum number of entries in the cache defaults to 32 and the maximum
number of blocks per cache entry has a default of 2, which has shown to
produce the best results on testing of ext4 and FAT filesystems.

The 'blkcache' command (enabled through CONFIG_CMD_BLOCK_CACHE) allows
changing these values and can be used to tune for a particular filesystem
layout.

Signed-off-by: Eric Nelson 
---
 disk/part.c|   2 +
 drivers/block/Kconfig  |  20 
 drivers/block/Makefile |   1 +
 drivers/block/blk-uclass.c |  13 +-
 drivers/block/blkcache.c   | 293 +
 include/blk.h  |  79 +++-
 6 files changed, 406 insertions(+), 2 deletions(-)
 create mode 100644 drivers/block/blkcache.c

diff --git a/disk/part.c b/disk/part.c
index 67d98fe..0aff954 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -268,6 +268,8 @@ void part_init(struct blk_desc *dev_desc)
const int n_ents = ll_entry_count(struct part_driver, part_driver);
struct part_driver *entry;
 
+   blkcache_invalidate(dev_desc->if_type, dev_desc->devnum);
+
dev_desc->part_type = PART_TYPE_UNKNOWN;
for (entry = drv; entry != drv + n_ents; entry++) {
int ret;
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index f35c4d4..0209b95 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -18,3 +18,23 @@ config DISK
  types can use this, such as AHCI/SATA. It does not provide any 
standard
  operations at present. The block device interface has not been 
converted
  to driver model.
+
+config BLOCK_CACHE
+   bool "Use block device cache"
+   default n
+   help
+ This option enables a disk-block cache for all block devices.
+ This is most useful when accessing filesystems under U-Boot since
+ it will prevent repeated reads from directory structures and other
+ filesystem data structures.
+
+config CMD_BLOCK_CACHE
+   bool "Include block device cache control command (blkcache)"
+   depends on BLOCK_CACHE
+   default y if BLOCK_CACHE
+   help
+ Enable the blkcache command, which can be used to control the
+ operation of the cache functions.
+ This is most useful when fine-tuning the operation of the cache
+ during development, but also allows the cache to be disabled when
+ it might hurt performance (e.g. when using the ums command).
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index b5c7ae1..b4cbb09 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -24,3 +24,4 @@ obj-$(CONFIG_IDE_SIL680) += sil680.o
 obj-$(CONFIG_SANDBOX) += sandbox.o
 obj-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o
 obj-$(CONFIG_SYSTEMACE) += systemace.o
+obj-$(CONFIG_BLOCK_CACHE) += blkcache.o
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 49df2a6..617db22 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -80,11 +80,20 @@ unsigned long blk_dread(struct blk_desc *block_dev, 
lbaint_t start,
 {
struct udevice *dev = block_dev->bdev;
const struct blk_ops *ops = blk_get_ops(dev);
+   ulong blks_read;
 
if (!ops->read)
return -ENOSYS;
 
-   return ops->read(dev, start, blkcnt, buffer);
+   if (blkcache_read(block_dev->if_type, block_dev->devnum,
+ start, blkcnt, block_dev->blksz, buffer))
+   return blkcnt;
+   blks_read = ops->read(dev, start, blkcnt, buffer);
+   if (blks_read == blkcnt)
+   blkcache_fill(block_dev->if_type, block_dev->devnum,
+ start, blkcnt, block_dev->blksz, buffer);
+
+   return blks_read;
 }
 
 unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start,
@@ -96,6 +105,7 @@ unsigned long blk_dwrite(struct blk_desc *block_dev, 
lbaint_t start,
if (!ops->write)
return -ENOSYS;
 
+   blkcache_invalidate(block_dev->if_type, block_dev->devnum);
return ops->write(dev, start, blkcnt, buffer);
 }
 
@@ -108,6 +118,7 @@ unsigned long