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 <ma...@denx.de> >>>>>>>> Cc: Tom Rini <tr...@konsulko.com> >>>>>>>> Cc: Simon Glass <s...@chromium.org> >>>>>>> >>>>>>> Reviewed-by: Tom Rini <tr...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot