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

[Marek Vasut]

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 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.

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