[U-Boot] [PATCH] [RFC] blk: Increase cache element size

[Marek Vasut]

On 08/17/2018 04:43 AM, Tom Rini wrote:
> On Thu, Aug 16, 2018 at 01:42:33PM +0200, Marek Vasut wrote:
>> On 08/15/2018 06:27 PM, Tom Rini wrote:
>>> On Wed, Aug 15, 2018 at 06:20:16PM +0200, Marek Vasut wrote:
>>>> On 08/15/2018 06:12 PM, Tom Rini wrote:
>>>>> On Wed, Aug 15, 2018 at 06:04:50PM +0200, Marek Vasut wrote:
>>>>>
>>>>>> On 08/15/2018 04:30 PM, Tom Rini wrote:
>>>>>>> On Wed, Aug 08, 2018 at 01:20:29PM +0200, Marek Vasut wrote:
>>>>>>>
>>>>>>>> Cache up to 4 kiB entries. 4 kiB is the default block size on ext4, yet
>>>>>>>> the underlying block layer devices usually report support for 512B . In
>>>>>>>> most cases, the 512B support is emulated (ie. SD cards, SSDs, USB sticks
>>>>>>>> etc.) and the real block size of those devices is much bigger.
>>>>>>>>
>>>>>>>> To avoid performance degradation with such devices and FS setup, bump
>>>>>>>> the maximum cache entry size to 4 kiB.
>>>>>>>>
>>>>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>>>>> Cc: Tom Rini <trini at konsulko.com>
>>>>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>>>>
>>>>>>> Reviewed-by: Tom Rini <trini at konsulko.com>
>>>>>>>
>>>>>>> I'll pick this up post v2018.09 if no one objects, thanks!
>>>>>>
>>>>>> I object. I was hoping there'd be some discussion on how to solve this
>>>>>> in a future-proof manner ... it's only a matter of time until someone
>>>>>> uses ext4 with 8k blocks on an SSD ...
>>>>>
>>>>> In general, sure?  In specific, mkfs.ext4 1.42.13 man page says 1/2/4KiB
>>>>> are the only valid values of block size, and based on having to whack
>>>>> this for some other projects it's pretty common for OpenEmbedded at
>>>>> least to spit out 1KiB block size images.
>>>>
>>>> OE spits 4k , that's how I triggered it,
>>>> meta/classes/image_types.bbclass:EXTRA_IMAGECMD_ext2 ?= "-i 4096"
>>>> meta/classes/image_types.bbclass:EXTRA_IMAGECMD_ext3 ?= "-i 4096"
>>>> meta/classes/image_types.bbclass:EXTRA_IMAGECMD_ext4 ?= "-i 4096"
>>>
>>> That's bytes-per-inode, I was referring to block size which is -b and
>>> dynamic unless specified.
>>
>> Right. Although, I think we mostly care about caching the inodes, not
>> blocks, since the inodes are accessed repeatedly.
> 
> OK, so it's the inode block.
> 
>>>>> So unless you know of cases
>>>>> today (or tomorrow, but not next year) where 8KiB is common or likely,
>>>>> we should probably just bump this for now and maybe make it a tunable so
>>>>> it's easily changed?
>>>>
>>>> It is already tunable, see blkcache config in blkcache command.
>>>>
>>>> But what I'd like to see is somehow the FS and the underlying storage
>>>> negotiating the best settings. Can we get the FS block size from the
>>>> block cache perspective ?
>>>
>>> Good questions that I don't have an answer to.
>>
>> What we can do is extend fs_devread() and blk_dread() with a new flag,
>> bool cachable, to allow FS and other upper layers which read block
>> devices to mark blocks that should specifically be cached as such. The
>> block cache would then cache the block no matter what size it has.
>>
>> The FS implementations should know which data should be cached because
>> they will be accessed repeatedly and which data are not, so they can set
>> the flag accordingly.
>>
>> The other users of blk_dread() would need to be examined. Possibly the
>> best solution right now to avoid problems would be to apply this patch
>> first. Then, second, add the flag to blk_dread() and set it accordingly
>> where applicable AND make the block cache cache blocks which are either
>> <= 8 kiB OR with cachable flag. Then finally, probably in next release,
>> drop the <= 8 kiB condition and cache only blocks which are flagged as
>> cachable to avoid polluting the block cache with useless small crumbs of
>> data.
>>
>> The downside is, the blk_dread() would look a bit asymmetrical compared
>> to the other functions with the cachable flag in it's parameters.
>>
>> Thoughts ?
> 
> Frankly, this sounds rather complicated when we might just need to
> increase the size of what we can cache a bit.  We aren't a long running
> facility and so long as it's tunable, it's also most likely being used
> in machines where we have enough DDR that we can a few more kilobytes.

It maybe increase the performance slightly though by not polluting the
cache with which shouldn't be there. Might even be useful in SPL.

-- 
Best regards,
Marek Vasut