[U-Boot] [RFC][PATCH] block: Enable block cache by default

[Tom Rini]

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 at 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 at denx.de>
> >>>>>> Cc: Simon Glass <sjg at chromium.org>
> >>>>>> Cc: Michal Simek <michal.simek at xilinx.com>
> >>>>>> Cc: Tom Rini <trini at konsulko.com>
> >>>>>> ---
> >>>>>>  drivers/block/Kconfig | 2 +-
> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> Reviewed-by: Simon Glass <sjg at 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>