block: Enable block cache by default
RFC PATCH 1/1 block: Enable block cache by default
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/block/Kconfig +1 -1
@@ -29,7 +29,7 @@ config SPL_BLK config BLOCK_CACHE bool "Use block device cache" - default n + default y help This option enables a disk-block cache for all block devices. This is most useful when accessing filesystems under U-Boot since
Signature · 1 line
2.16.2
RFC PATCH 1/1 block: Enable block cache by default
Quoted 8 lines · Marek Vasut
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
Quoted 7 lines
Signed-off-by Marek Vasut <[email protected]>Cc Simon Glass <[email protected]>Cc Michal Simek <[email protected]>Cc Tom Rini <[email protected]>--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
RFC PATCH 1/1 block: Enable block cache by default
Quoted 17 lines · Simon Glass
Quoted 8 lines · Marek Vasut
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
Quoted 7 lines
Signed-off-by Marek Vasut <[email protected]>Cc Simon Glass <[email protected]>Cc Michal Simek <[email protected]>Cc Tom Rini <[email protected]>--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 2 lines
Reviewed-by Simon Glass <[email protected]>
I was hoping to get some feedback ?
Signature · 2 lines
Best regards, Marek Vasut
RFC PATCH 1/1 block: Enable block cache by default
Quoted 18 lines · Marek Vasut
Quoted 17 lines · Simon Glass
Quoted 15 lines · Marek Vasut
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
Signed-off-by Marek Vasut <[email protected]>Cc Simon Glass <[email protected]>Cc Michal Simek <[email protected]>Cc Tom Rini <[email protected]>--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 3 lines
Quoted 1 line
Reviewed-by Simon Glass <[email protected]>I was hoping to get some feedback ?
So, as I was asking on IRC, can you show the code paths where this gets used outside of CONFIG_BLK and then really ext4/fat/btrfs as I do in my patch?
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180519/0b9220cf/attachment.sig>
RFC PATCH 1/1 block: Enable block cache by default
Quoted 19 lines · Tom Rini
Quoted 18 lines · Marek Vasut
Quoted 17 lines · Simon Glass
Quoted 15 lines · Marek Vasut
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
Signed-off-by Marek Vasut <[email protected]>Cc Simon Glass <[email protected]>Cc Michal Simek <[email protected]>Cc Tom Rini <[email protected]>--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Reviewed-by Simon Glass <[email protected]>I was hoping to get some feedback ?
So, as I was asking on IRC, can you show the code paths where this gets used outside of
CONFIG_BLKand then really ext4/fat/btrfs as I do in my patch?
Can you summarize that discussion for everyone who was not on IRC at that point ?
Signature · 2 lines
Best regards, Marek Vasut
RFC PATCH 1/1 block: Enable block cache by default
Quoted 22 lines · Marek Vasut
Quoted 21 lines · Tom Rini
Quoted 20 lines · Marek Vasut
Quoted 18 lines · Simon Glass
Quoted 8 lines · Marek Vasut
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
Quoted 7 lines
Signed-off-by Marek Vasut <[email protected]>Cc Simon Glass <[email protected]>Cc Michal Simek <[email protected]>Cc Tom Rini <[email protected]>--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Reviewed-by Simon Glass <[email protected]>
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
I was hoping to get some feedback ?
So, as I was asking on IRC, can you show the code paths where this gets used outside of
CONFIG_BLKand then really ext4/fat/btrfs as I do in my patch?Can you summarize that discussion for everyone who was not on IRC at that point ?
So I posted https://patchwork.ozlabs.org/patch/913266/ and it depends on BLK, as without BLK it does compile but isn't useful, but also isn't wholly discarded (due to the invalidate call in disk/part.c). AFIACT from a quick read of the code, block cache is only useful on filesystems that reside on block devices. It won't help with "just" MMC reads for example. So we should only enable it by default in the case of filesystems that are usually on block devices being enabled.
But I didn't dig through the code hard enough to see if it would be useful on say UBIFS or if I'm wrong about it not being in the code path of things like say NAND.
But I also don't think just default y in all cases is right as that's adding non-trivial mounts of code on all of the platforms that don't / won't make use of it.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180519/05771c57/attachment.sig>
RFC PATCH 1/1 block: Enable block cache by default
Quoted 29 lines · Tom Rini
Quoted 28 lines · Marek Vasut
Quoted 26 lines · Tom Rini
Quoted 18 lines · Marek Vasut
Quoted 17 lines · Simon Glass
Quoted 15 lines · Marek Vasut
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
Signed-off-by Marek Vasut <[email protected]>Cc Simon Glass <[email protected]>Cc Michal Simek <[email protected]>Cc Tom Rini <[email protected]>--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 3 lines
Quoted 1 line
Reviewed-by Simon Glass <[email protected]>I was hoping to get some feedback ?
So, as I was asking on IRC, can you show the code paths where this gets used outside of
CONFIG_BLKand then really ext4/fat/btrfs as I do in my patch?
Quoted 6 lines
Quoted 2 lines
Can you summarize that discussion for everyone who was not on IRC at that point ?
So I posted https://patchwork.ozlabs.org/patch/913266/ and it depends on BLK, as without BLK it does compile but isn't useful, but also isn't wholly discarded (due to the invalidate call in
disk/part.c).
Maybe that is what needs fixing and then this patch can be applied ?
Quoted 5 lines
AFIACT
from a quick read of the code, block cache is only useful on filesystems that reside on block devices. It won't help with "just" MMC reads for example. So we should only enable it by default in the case of filesystems that are usually on block devices being enabled.
I wonder if this not helping with raw block reads is fine or not. Thoughts ?
Quoted 3 lines
But I didn't dig through the code hard enough to see if it would be useful on say UBIFS or if I'm wrong about it not being in the code path of things like say NAND.
I don't see why this won't be useful on UBI/UBIFS . It is probably just not implemented yet.
Quoted 3 lines
But I also don't think just default y in all cases is right as that's adding non-trivial mounts of code on all of the platforms that don't / won't make use of it.
So it should be discarded if there are no users ?
Signature · 2 lines
Best regards, Marek Vasut
RFC PATCH 1/1 block: Enable block cache by default
Quoted 30 lines · Marek Vasut
Quoted 29 lines · Tom Rini
Quoted 28 lines · Marek Vasut
Quoted 26 lines · Tom Rini
Quoted 18 lines · Marek Vasut
Quoted 17 lines · Simon Glass
Quoted 15 lines · Marek Vasut
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
Signed-off-by Marek Vasut <[email protected]>Cc Simon Glass <[email protected]>Cc Michal Simek <[email protected]>Cc Tom Rini <[email protected]>--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 3 lines
Quoted 1 line
Reviewed-by Simon Glass <[email protected]>I was hoping to get some feedback ?
So, as I was asking on IRC, can you show the code paths where this gets used outside of
CONFIG_BLKand then really ext4/fat/btrfs as I do in my patch?
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Can you summarize that discussion for everyone who was not on IRC at that point ?
So I posted https://patchwork.ozlabs.org/patch/913266/ and it depends on BLK, as without BLK it does compile but isn't useful, but also isn't wholly discarded (due to the invalidate call in
disk/part.c).Maybe that is what needs fixing and then this patch can be applied ?
No, that's just the nature of the functionality. We have an option for block cache for the DM based block class.
Quoted 8 lines
Quoted 5 lines
AFIACT
from a quick read of the code, block cache is only useful on filesystems that reside on block devices. It won't help with "just" MMC reads for example. So we should only enable it by default in the case of filesystems that are usually on block devices being enabled.I wonder if this not helping with raw block reads is fine or not. Thoughts ?
So you got me to look at the code and it should be more widely enabled that I suggested as it is used on block devices even on raw reads.
Quoted 6 lines
Quoted 3 lines
But I didn't dig through the code hard enough to see if it would be useful on say UBIFS or if I'm wrong about it not being in the code path of things like say NAND.
I don't see why this won't be useful on UBI/UBIFS . It is probably just not implemented yet.
It's not useful outside of "block" devices, of which NAND (and SPI and NOR and so forth) are not.
Quoted 5 lines
Quoted 3 lines
But I also don't think just default y in all cases is right as that's adding non-trivial mounts of code on all of the platforms that don't / won't make use of it.
So it should be discarded if there are no users ?
Yes, it is. Based on confirming it is used on raw block reads, I don't think the cases I saw with growth, when we have BLK enabled, were a problem.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180520/b07ecae6/attachment.sig>
RFC PATCH 1/1 block: Enable block cache by default
Quoted 34 lines · Tom Rini
Quoted 33 lines · Marek Vasut
Quoted 32 lines · Tom Rini
Quoted 30 lines · Marek Vasut
Quoted 19 lines · Tom Rini
Quoted 18 lines · Marek Vasut
Quoted 17 lines · Simon Glass
Quoted 15 lines · Marek Vasut
The recent ext4 cache discussion would indicate that the block cache is a desired feature, yet hidden and not enabled most of the time. Enable the block cache by default since it provides significant block device access performance improvement and if there are some users who cannot enable it ie. due to size limitations, those should disable it explicitly in their board config.
Signed-off-by Marek Vasut <[email protected]>Cc Simon Glass <[email protected]>Cc Michal Simek <[email protected]>Cc Tom Rini <[email protected]>--- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Reviewed-by Simon Glass <[email protected]>I was hoping to get some feedback ?
So, as I was asking on IRC, can you show the code paths where this gets used outside of
CONFIG_BLKand then really ext4/fat/btrfs as I do in my patch?Can you summarize that discussion for everyone who was not on IRC at that point ?
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
So I posted https://patchwork.ozlabs.org/patch/913266/ and it depends on BLK, as without BLK it does compile but isn't useful, but also isn't wholly discarded (due to the invalidate call in
disk/part.c).Maybe that is what needs fixing and then this patch can be applied ?
No, that's just the nature of the functionality. We have an option for block cache for the DM based block class.
Then it should probably be enabled by default if block class is enabled ? :)
Quoted 11 lines
Quoted 8 lines
Quoted 5 lines
AFIACT
from a quick read of the code, block cache is only useful on filesystems that reside on block devices. It won't help with "just" MMC reads for example. So we should only enable it by default in the case of filesystems that are usually on block devices being enabled.I wonder if this not helping with raw block reads is fine or not. Thoughts ?
So you got me to look at the code and it should be more widely enabled that I suggested as it is used on block devices even on raw reads.
Good!
Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
But I didn't dig through the code hard enough to see if it would be useful on say UBIFS or if I'm wrong about it not being in the code path of things like say NAND.
I don't see why this won't be useful on UBI/UBIFS . It is probably just not implemented yet.
It's not useful outside of "block" devices, of which NAND (and SPI and NOR and so forth) are not.
Don't we have mtdblock or somesuch ? (caching block access to MTD devices) ?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
But I also don't think just default y in all cases is right as that's adding non-trivial mounts of code on all of the platforms that don't / won't make use of it.
So it should be discarded if there are no users ?
Yes, it is. Based on confirming it is used on raw block reads, I don't think the cases I saw with growth, when we have BLK enabled, were a problem.
Errrr, can you rephrase ?
Signature · 2 lines
Best regards, Marek Vasut