On Fri, 6 Oct 2023 20:10:13 +0300 ValdikSS via Grub-devel <grub-devel@gnu.org> wrote:
> Increase the value from 63 to speed up reading process. > > This commit increases two limits: the low-level int 13h reading code > and a high-level reading code with disk cache. > 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. > > According to Wikipedia and OSDev, the upper safe value for LBA > read using IBM/MS INT13 Extensions is 127 sectors due to the > limitations of some BIOSes. > GRUB already enforced the limit, but it was no-op due to other > constraints. This value is also used in syslinux. > > As we're now reading up to 127 sectors of 512 bytes, we need to be able > to store 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). Much better explanation, thanks. > > See: > https://en.wikipedia.org/wiki/INT_13H#INT_13h_AH=42h:_Extended_Read_Sectors_From_Drive > See: > https://wiki.osdev.org/Disk_access_using_the_BIOS_(INT_13h)#LBA_in_Extended_Mode > See: > https://github.com/rhboot/grub2/blob/10f8ffc133553209ec1ddaadc6f4a8a25d3dea4e/grub-core/disk/i386/pc/biosdisk.c#L434 Commit messages should not use links to downstream projects. The commit hash referred to in that link does not exist in GRUB master. Here's a more appropriate link: https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/disk/i386/pc/biosdisk.c?h=grub-2.12-rc1#n430 > See: > https://github.com/geneC/syslinux/blob/5e426532210bb830d2d7426eb8d8c154d9dfcba6/core/fs/diskio_bios.c#L349 I would also change this to link to the official syslinux repository, not an unofficial mirror that may be gone tomorrow. I'd use this link instead: https://repo.or.cz/syslinux.git/blob/refs/tags/syslinux-6.03:/core/fs/diskio_bios.c#l349 Also, the commit message could be improved by using the reference syntax (I think its markdown). This will show where the link is relevant. So this link would be [n], where n is an integer usually starting from 1. Then above I would use "syslinux[n]". This way its more clear in context why this link is here. It would be nice to have all the links this way. Look at other commit messages for guidance. Glenn > > Signed-off-by: ValdikSS <i...@valdikss.org.ru> > --- > 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 > #define GRUB_DISK_CACHE_SIZE (1 << GRUB_DISK_CACHE_BITS) > > #define GRUB_DISK_MAX_MAX_AGGLOMERATE ((1 << (30 - GRUB_DISK_CACHE_BITS - > GRUB_DISK_SECTOR_BITS)) - 1) > > /* Maximum number of sectors to read in LBA mode at once */ > -#define GRUB_DISK_MAX_LBA_SECTORS 63 > +#define GRUB_DISK_MAX_LBA_SECTORS 127 > > /* Return value of grub_disk_native_sectors() in case disk size is unknown. > */ > #define GRUB_DISK_SIZE_UNKNOWN 0xffffffffffffffffULL _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
