Re: Incorrect comment in disk.h (and bigger cache) ?

[Daniel Kiper]

On Thu, Feb 09, 2023 at 01:24:52AM +0100, Mihai Moldovan wrote:
> * On 2/9/23 01:05, Benjamin Herrenschmidt wrote:
> > Stumbled upon this:
> >
> > /* The size of a disk cache in 512B units. Must be at least as big as the
> >    largest supported sector size, currently 16K.  */
> > #define GRUB_DISK_CACHE_BITS        6
> > #define GRUB_DISK_CACHE_SIZE        (1 << GRUB_DISK_CACHE_BITS)
> >
> > It's me or somebody had a bad day with math ? :-) It's 32K no ?
>
> I believe that you merely misinterpreted the comment, though it would be good 
> to
> clarify it. The largest currently supported sector size seems to be 16 KByte,
> not the cache size. This said, the comment might also be out of date, not sure
> if the sector size is actually still limited any longer. biosdisk for instance
> should give hints to that (although, naturally, that is not the only place 
> where
> this is used).

After looking at this once again I agree with Mihai. Though I am not
native speaker. So, you have to take it with a grain of salt... :-)
Anyway, I think the comment should be clarified...

> > I'm tempted to increase this to speed things up on UEFI especially, either
> > that or maybe changing grub_fshelp_read_file() so that get_block can
> > optionally return a count of contiguous blocks so we can issue larger
> > requests ?
>
> Is that really necessary? Note that this is also used for legacy MBR/BIOS
> systems and I faintly remember places using that, which may do so in a fixed
> way, so I advise to check it well if you really intend to change it.

I concur. I think I would do this only for UEFI systems first. If at all
we need to increase this value. How often the GRUB reads the same files
from the disk? Did you notice faster loads/boots when you increase the
disk cache size?

Daniel