* [U-Boot] [RFC][PATCH] block: Enable block cache by default @ 2018-05-18 9:22 Marek Vasut 2018-05-19 14:36 ` Simon Glass 0 siblings, 1 reply; 9+ messages in thread From: Marek Vasut @ 2018-05-18 9:22 UTC (permalink / raw) To: u-boot 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 <marex@denx.de> Cc: Simon Glass <sjg@chromium.org> Cc: Michal Simek <michal.simek@xilinx.com> Cc: Tom Rini <trini@konsulko.com> --- drivers/block/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 15fd1bcb2b..832d626bd4 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -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 -- 2.16.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [RFC][PATCH] block: Enable block cache by default 2018-05-18 9:22 [U-Boot] [RFC][PATCH] block: Enable block cache by default Marek Vasut @ 2018-05-19 14:36 ` Simon Glass 2018-05-19 18:20 ` Marek Vasut 0 siblings, 1 reply; 9+ messages in thread From: Simon Glass @ 2018-05-19 14:36 UTC (permalink / raw) To: u-boot On 18 May 2018 at 03:22, Marek Vasut <marex@denx.de> wrote: > > 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 <marex@denx.de> > Cc: Simon Glass <sjg@chromium.org> > Cc: Michal Simek <michal.simek@xilinx.com> > Cc: Tom Rini <trini@konsulko.com> > --- > drivers/block/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC][PATCH] block: Enable block cache by default 2018-05-19 14:36 ` Simon Glass @ 2018-05-19 18:20 ` Marek Vasut 2018-05-19 20:50 ` Tom Rini 0 siblings, 1 reply; 9+ messages in thread From: Marek Vasut @ 2018-05-19 18:20 UTC (permalink / raw) To: u-boot On 05/19/2018 04:36 PM, Simon Glass wrote: > On 18 May 2018 at 03:22, Marek Vasut <marex@denx.de> wrote: >> >> 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 <marex@denx.de> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Michal Simek <michal.simek@xilinx.com> >> Cc: Tom Rini <trini@konsulko.com> >> --- >> drivers/block/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > I was hoping to get some feedback ? -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC][PATCH] block: Enable block cache by default 2018-05-19 18:20 ` Marek Vasut @ 2018-05-19 20:50 ` Tom Rini 2018-05-19 21:39 ` Marek Vasut 0 siblings, 1 reply; 9+ messages in thread From: Tom Rini @ 2018-05-19 20:50 UTC (permalink / raw) To: u-boot On Sat, May 19, 2018 at 08:20:30PM +0200, Marek Vasut wrote: > On 05/19/2018 04:36 PM, Simon Glass wrote: > > On 18 May 2018 at 03:22, Marek Vasut <marex@denx.de> wrote: > >> > >> 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 <marex@denx.de> > >> Cc: Simon Glass <sjg@chromium.org> > >> Cc: Michal Simek <michal.simek@xilinx.com> > >> Cc: Tom Rini <trini@konsulko.com> > >> --- > >> drivers/block/Kconfig | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > 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? -- 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> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC][PATCH] block: Enable block cache by default 2018-05-19 20:50 ` Tom Rini @ 2018-05-19 21:39 ` Marek Vasut 2018-05-19 22:29 ` Tom Rini 0 siblings, 1 reply; 9+ messages in thread From: Marek Vasut @ 2018-05-19 21:39 UTC (permalink / raw) To: u-boot On 05/19/2018 10:50 PM, Tom Rini wrote: > On Sat, May 19, 2018 at 08:20:30PM +0200, Marek Vasut wrote: >> On 05/19/2018 04:36 PM, Simon Glass wrote: >>> On 18 May 2018 at 03:22, Marek Vasut <marex@denx.de> wrote: >>>> >>>> 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 <marex@denx.de> >>>> Cc: Simon Glass <sjg@chromium.org> >>>> Cc: Michal Simek <michal.simek@xilinx.com> >>>> Cc: Tom Rini <trini@konsulko.com> >>>> --- >>>> drivers/block/Kconfig | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> 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? Can you summarize that discussion for everyone who was not on IRC at that point ? -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC][PATCH] block: Enable block cache by default 2018-05-19 21:39 ` Marek Vasut @ 2018-05-19 22:29 ` Tom Rini 2018-05-19 23:39 ` Marek Vasut 0 siblings, 1 reply; 9+ messages in thread From: Tom Rini @ 2018-05-19 22:29 UTC (permalink / raw) To: u-boot On Sat, May 19, 2018 at 11:39:38PM +0200, Marek Vasut wrote: > On 05/19/2018 10:50 PM, Tom Rini wrote: > > On Sat, May 19, 2018 at 08:20:30PM +0200, Marek Vasut wrote: > >> On 05/19/2018 04:36 PM, Simon Glass wrote: > >>> On 18 May 2018 at 03:22, Marek Vasut <marex@denx.de> wrote: > >>>> > >>>> 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 <marex@denx.de> > >>>> Cc: Simon Glass <sjg@chromium.org> > >>>> Cc: Michal Simek <michal.simek@xilinx.com> > >>>> Cc: Tom Rini <trini@konsulko.com> > >>>> --- > >>>> drivers/block/Kconfig | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> Reviewed-by: Simon Glass <sjg@chromium.org> > >> > >> 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? > > 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. -- 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> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC][PATCH] block: Enable block cache by default 2018-05-19 22:29 ` Tom Rini @ 2018-05-19 23:39 ` Marek Vasut 2018-05-21 1:48 ` Tom Rini 0 siblings, 1 reply; 9+ messages in thread From: Marek Vasut @ 2018-05-19 23:39 UTC (permalink / raw) To: u-boot On 05/20/2018 12:29 AM, Tom Rini wrote: > On Sat, May 19, 2018 at 11:39:38PM +0200, Marek Vasut wrote: >> On 05/19/2018 10:50 PM, Tom Rini wrote: >>> On Sat, May 19, 2018 at 08:20:30PM +0200, Marek Vasut wrote: >>>> On 05/19/2018 04:36 PM, Simon Glass wrote: >>>>> On 18 May 2018 at 03:22, Marek Vasut <marex@denx.de> wrote: >>>>>> >>>>>> 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 <marex@denx.de> >>>>>> Cc: Simon Glass <sjg@chromium.org> >>>>>> Cc: Michal Simek <michal.simek@xilinx.com> >>>>>> Cc: Tom Rini <trini@konsulko.com> >>>>>> --- >>>>>> drivers/block/Kconfig | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>> >>>> 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? >> >> 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 ? > 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 ? > 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. > 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 ? -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC][PATCH] block: Enable block cache by default 2018-05-19 23:39 ` Marek Vasut @ 2018-05-21 1:48 ` Tom Rini 2018-05-21 2:17 ` Marek Vasut 0 siblings, 1 reply; 9+ messages in thread From: Tom Rini @ 2018-05-21 1:48 UTC (permalink / raw) To: u-boot On Sun, May 20, 2018 at 01:39:07AM +0200, Marek Vasut wrote: > On 05/20/2018 12:29 AM, Tom Rini wrote: > > On Sat, May 19, 2018 at 11:39:38PM +0200, Marek Vasut wrote: > >> On 05/19/2018 10:50 PM, Tom Rini wrote: > >>> On Sat, May 19, 2018 at 08:20:30PM +0200, Marek Vasut wrote: > >>>> On 05/19/2018 04:36 PM, Simon Glass wrote: > >>>>> On 18 May 2018 at 03:22, Marek Vasut <marex@denx.de> wrote: > >>>>>> > >>>>>> 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 <marex@denx.de> > >>>>>> Cc: Simon Glass <sjg@chromium.org> > >>>>>> Cc: Michal Simek <michal.simek@xilinx.com> > >>>>>> Cc: Tom Rini <trini@konsulko.com> > >>>>>> --- > >>>>>> drivers/block/Kconfig | 2 +- > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> Reviewed-by: Simon Glass <sjg@chromium.org> > >>>> > >>>> 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? > >> > >> 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. > > 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. > > 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. > > 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. -- 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> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC][PATCH] block: Enable block cache by default 2018-05-21 1:48 ` Tom Rini @ 2018-05-21 2:17 ` Marek Vasut 0 siblings, 0 replies; 9+ messages in thread From: Marek Vasut @ 2018-05-21 2:17 UTC (permalink / raw) To: u-boot On 05/21/2018 03:48 AM, Tom Rini wrote: > On Sun, May 20, 2018 at 01:39:07AM +0200, Marek Vasut wrote: >> On 05/20/2018 12:29 AM, Tom Rini wrote: >>> On Sat, May 19, 2018 at 11:39:38PM +0200, Marek Vasut wrote: >>>> On 05/19/2018 10:50 PM, Tom Rini wrote: >>>>> On Sat, May 19, 2018 at 08:20:30PM +0200, Marek Vasut wrote: >>>>>> On 05/19/2018 04:36 PM, Simon Glass wrote: >>>>>>> On 18 May 2018 at 03:22, Marek Vasut <marex@denx.de> wrote: >>>>>>>> >>>>>>>> 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 <marex@denx.de> >>>>>>>> Cc: Simon Glass <sjg@chromium.org> >>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com> >>>>>>>> Cc: Tom Rini <trini@konsulko.com> >>>>>>>> --- >>>>>>>> drivers/block/Kconfig | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>> >>>>>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>>>> >>>>>> 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? >>>> >>>> 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. Then it should probably be enabled by default if block class is enabled ? :) >>> 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! >>> 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) ? >>> 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 ? -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-05-21 2:17 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-18 9:22 [U-Boot] [RFC][PATCH] block: Enable block cache by default Marek Vasut 2018-05-19 14:36 ` Simon Glass 2018-05-19 18:20 ` Marek Vasut 2018-05-19 20:50 ` Tom Rini 2018-05-19 21:39 ` Marek Vasut 2018-05-19 22:29 ` Tom Rini 2018-05-19 23:39 ` Marek Vasut 2018-05-21 1:48 ` Tom Rini 2018-05-21 2:17 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.