Re: [PATCH 2/2] disk: increase sector size up to 127 for LBA reads

[ValdikSS]

On 05.10.2023 22:11, Glenn Washburn wrote:
> > According to Wikipedia and various sources, the recommended
> > value for LBA read using IBM/MS INT13 Extensions is 127 sectors.
>
> Please cite references. I'm not seeing that on Wikipedia or osdev.org.
> I do see where Wikipedia says that some Phoenix bioses are limited to a
> maximum of 127 sectors, but I'm not sure that's a recommendation.

Done, check v2. Also checked syslinux code, they also have 127.

> > ---
> >   include/grub/disk.h | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/grub/disk.h b/include/grub/disk.h
> > index be032a72c..608deb034 100644
> > --- a/include/grub/disk.h
> > +++ b/include/grub/disk.h
> > @@ -184,14 +184,14 @@ typedef struct grub_disk_memberlist 
> > *grub_disk_memberlist_t;
> >   #define GRUB_MDRAID_MAX_DISKS 4096
> >   
> >   /* 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
> > +   largest supported sector size, currently 64K.  */
> > +#define GRUB_DISK_CACHE_BITS   7
>
> This change is not reflected in the commit message and is not obvious
> why this is changed. Is it necessary due to the change below or
> additional performance improvement?

The disk cache imposes an overall limitation of a higher-layer reading 
code. The original comment regarding 16K is incorrect, it was 512<<6 = 
32768, and now it is 512<<7 = 65536.

As we're now reading up to 127 sectors of 512 bytes, we need to be able 
to put in the cache up to 65024 bytes. Without this change, GRUB 
wouldn't try to read more than 64 sectors at once (even if the lower 
reading layer allows it).

OpenPGP_signature
Description: OpenPGP digital signature