public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
@ 2018-08-08 11:20 Marek Vasut
  2018-08-15 14:30 ` Tom Rini
  2019-01-16  2:39 ` [U-Boot] [U-Boot,RFC] " Tom Rini
  0 siblings, 2 replies; 10+ messages in thread
From: Marek Vasut @ 2018-08-08 11:20 UTC (permalink / raw)
  To: u-boot

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@denx.de>
Cc: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>
---
 drivers/block/blkcache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/blkcache.c b/drivers/block/blkcache.c
index 294511fcdb..1fa64989d3 100644
--- a/drivers/block/blkcache.c
+++ b/drivers/block/blkcache.c
@@ -24,7 +24,7 @@ struct block_cache_node {
 static LIST_HEAD(block_cache);
 
 static struct block_cache_stats _stats = {
-	.max_blocks_per_entry = 2,
+	.max_blocks_per_entry = 8,
 	.max_entries = 32
 };
 
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
  2018-08-08 11:20 [U-Boot] [PATCH] [RFC] blk: Increase cache element size Marek Vasut
@ 2018-08-15 14:30 ` Tom Rini
  2018-08-15 16:04   ` Marek Vasut
  2019-01-16  2:39 ` [U-Boot] [U-Boot,RFC] " Tom Rini
  1 sibling, 1 reply; 10+ messages in thread
From: Tom Rini @ 2018-08-15 14:30 UTC (permalink / raw)
  To: u-boot

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@denx.de>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

I'll pick this up post v2018.09 if no one objects, thanks!

-- 
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/20180815/4a89d264/attachment.sig>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
  2018-08-15 14:30 ` Tom Rini
@ 2018-08-15 16:04   ` Marek Vasut
  2018-08-15 16:12     ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2018-08-15 16:04 UTC (permalink / raw)
  To: u-boot

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@denx.de>
>> Cc: Tom Rini <trini@konsulko.com>
>> Cc: Simon Glass <sjg@chromium.org>
> 
> Reviewed-by: Tom Rini <trini@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 ...

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
  2018-08-15 16:04   ` Marek Vasut
@ 2018-08-15 16:12     ` Tom Rini
  2018-08-15 16:20       ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2018-08-15 16:12 UTC (permalink / raw)
  To: u-boot

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@denx.de>
> >> Cc: Tom Rini <trini@konsulko.com>
> >> Cc: Simon Glass <sjg@chromium.org>
> > 
> > Reviewed-by: Tom Rini <trini@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.  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?

-- 
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/20180815/68c58710/attachment.sig>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
  2018-08-15 16:12     ` Tom Rini
@ 2018-08-15 16:20       ` Marek Vasut
  2018-08-15 16:27         ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2018-08-15 16:20 UTC (permalink / raw)
  To: u-boot

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@denx.de>
>>>> Cc: Tom Rini <trini@konsulko.com>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>
>>> Reviewed-by: Tom Rini <trini@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"

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

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
  2018-08-15 16:20       ` Marek Vasut
@ 2018-08-15 16:27         ` Tom Rini
  2018-08-16 11:42           ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2018-08-15 16:27 UTC (permalink / raw)
  To: u-boot

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@denx.de>
> >>>> Cc: Tom Rini <trini@konsulko.com>
> >>>> Cc: Simon Glass <sjg@chromium.org>
> >>>
> >>> Reviewed-by: Tom Rini <trini@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.

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

-- 
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/20180815/61c269cb/attachment.sig>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
  2018-08-15 16:27         ` Tom Rini
@ 2018-08-16 11:42           ` Marek Vasut
  2018-08-17  2:43             ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2018-08-16 11:42 UTC (permalink / raw)
  To: u-boot

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@denx.de>
>>>>>> Cc: Tom Rini <trini@konsulko.com>
>>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>>
>>>>> Reviewed-by: Tom Rini <trini@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.

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

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
  2018-08-16 11:42           ` Marek Vasut
@ 2018-08-17  2:43             ` Tom Rini
  2018-08-17 10:13               ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2018-08-17  2:43 UTC (permalink / raw)
  To: u-boot

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@denx.de>
> >>>>>> Cc: Tom Rini <trini@konsulko.com>
> >>>>>> Cc: Simon Glass <sjg@chromium.org>
> >>>>>
> >>>>> Reviewed-by: Tom Rini <trini@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.

-- 
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/20180816/54283ee9/attachment.sig>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] [RFC] blk: Increase cache element size
  2018-08-17  2:43             ` Tom Rini
@ 2018-08-17 10:13               ` Marek Vasut
  0 siblings, 0 replies; 10+ messages in thread
From: Marek Vasut @ 2018-08-17 10:13 UTC (permalink / raw)
  To: u-boot

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@denx.de>
>>>>>>>> Cc: Tom Rini <trini@konsulko.com>
>>>>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>>>>
>>>>>>> Reviewed-by: Tom Rini <trini@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot,RFC] blk: Increase cache element size
  2018-08-08 11:20 [U-Boot] [PATCH] [RFC] blk: Increase cache element size Marek Vasut
  2018-08-15 14:30 ` Tom Rini
@ 2019-01-16  2:39 ` Tom Rini
  1 sibling, 0 replies; 10+ messages in thread
From: Tom Rini @ 2019-01-16  2:39 UTC (permalink / raw)
  To: u-boot

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@denx.de>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
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/20190115/4476c498/attachment.sig>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-01-16  2:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-08 11:20 [U-Boot] [PATCH] [RFC] blk: Increase cache element size Marek Vasut
2018-08-15 14:30 ` Tom Rini
2018-08-15 16:04   ` Marek Vasut
2018-08-15 16:12     ` Tom Rini
2018-08-15 16:20       ` Marek Vasut
2018-08-15 16:27         ` Tom Rini
2018-08-16 11:42           ` Marek Vasut
2018-08-17  2:43             ` Tom Rini
2018-08-17 10:13               ` Marek Vasut
2019-01-16  2:39 ` [U-Boot] [U-Boot,RFC] " Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox