From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Thu, 16 Aug 2018 22:43:30 -0400 Subject: [U-Boot] [PATCH] [RFC] blk: Increase cache element size In-Reply-To: <6f21f998-5d7a-3aa5-6856-d16a3e2f1285@denx.de> References: <20180808112029.3866-1-marex@denx.de> <20180815143008.GJ30947@bill-the-cat> <4e77ae7a-d5cb-159a-da1c-d16dfa82b9c1@denx.de> <20180815161259.GN30947@bill-the-cat> <0db4d121-efb2-5f71-fc15-b5116695e322@denx.de> <20180815162729.GO30947@bill-the-cat> <6f21f998-5d7a-3aa5-6856-d16a3e2f1285@denx.de> Message-ID: <20180817024330.GF30947@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > >>>>>> Cc: Tom Rini > >>>>>> Cc: Simon Glass > >>>>> > >>>>> Reviewed-by: Tom Rini > >>>>> > >>>>> 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. -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: