It's been over two weeks and a quick Rb would be great, since this patch is relatively trivial and it would fix a bug. Again, if I'm doing anything wrong, please let me know. Thanks, Pedro On Mon, Nov 1, 2021 at 9:29 PM Pedro Falcato wrote: > Quick PS: I CC'd the edk2-platforms stewards on the patch because there's > no co-maintainer, and from what I gathered, that's what I'm supposed to do. > If I'm mistaken, please let me know. > > Thanks, > Pedro > > On Fri, Oct 29, 2021 at 2:04 AM Pedro Falcato via groups.io > wrote: > >> Uninitialized extents are special extents that have blocks allocated, >> but are specified as uninitialized and therefore, reads behave the >> same as file holes (reads 0), while writes already have blocks allocated. >> >> Cc: Leif Lindholm >> Cc: Michael D Kinney >> Signed-off-by: Pedro Falcato >> --- >> Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 8 ++++++++ >> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 21 +++++++++++++++++++++ >> Features/Ext4Pkg/Ext4Dxe/Extents.c | 24 ++++++++++++++++++++++-- >> Features/Ext4Pkg/Ext4Dxe/Inode.c | 14 +++++++++++--- >> 4 files changed, 62 insertions(+), 5 deletions(-) >> >> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h >> b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h >> index 070eb5a9c8..756b1bbe10 100644 >> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h >> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h >> @@ -448,6 +448,14 @@ typedef struct { >> UINT32 eb_checksum; >> } EXT4_EXTENT_TAIL; >> >> +/** >> + * EXT4 has this feature called uninitialized extents: >> + * An extent has a maximum of 32768 blocks (2^15 or 1 << 15). >> + * When we find an extent with > 32768 blocks, this extent is called >> uninitialized. >> + * Long story short, it's an extent that behaves as a file hole but has >> blocks already allocated. >> + */ >> +#define EXT4_EXTENT_MAX_INITIALIZED (1 << 15) >> + >> typedef UINT64 EXT4_BLOCK_NR; >> typedef UINT32 EXT4_INO_NR; >> >> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h >> b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h >> index a9b932ed52..1d9a4ac6ba 100644 >> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h >> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h >> @@ -1131,4 +1131,25 @@ Ext4SuperblockCheckMagic ( >> IN EFI_BLOCK_IO_PROTOCOL *BlockIo >> ); >> >> +/** >> + Check if the extent is uninitialized >> + >> + @param[in] Extent Pointer to the EXT4_EXTENT >> + >> + @returns True if uninitialized, else false. >> +**/ >> +#define EXT4_EXTENT_IS_UNINITIALIZED(Extent) ((Extent)->ee_len > >> EXT4_EXTENT_MAX_INITIALIZED) >> + >> +/** >> + Retrieves the extent's length, dealing with uninitialized extents in >> the process. >> + >> + @param[in] Extent Pointer to the EXT4_EXTENT >> + >> + @returns Extent's length, in filesystem blocks. >> +**/ >> +EXT4_BLOCK_NR >> +Ext4GetExtentLength ( >> + IN CONST EXT4_EXTENT *Extent >> + ); >> + >> #endif >> diff --git a/Features/Ext4Pkg/Ext4Dxe/Extents.c >> b/Features/Ext4Pkg/Ext4Dxe/Extents.c >> index 5fa2fe098d..1ae175f417 100644 >> --- a/Features/Ext4Pkg/Ext4Dxe/Extents.c >> +++ b/Features/Ext4Pkg/Ext4Dxe/Extents.c >> @@ -332,7 +332,7 @@ Ext4GetExtent ( >> return EFI_NO_MAPPING; >> } >> >> - if (!(LogicalBlock >= Ext->ee_block && Ext->ee_block + Ext->ee_len > >> LogicalBlock)) { >> + if (!(LogicalBlock >= Ext->ee_block && Ext->ee_block + >> Ext4GetExtentLength (Ext) > LogicalBlock)) { >> // This extent does not cover the block >> if (Buffer != NULL) { >> FreePool (Buffer); >> @@ -413,7 +413,7 @@ Ext4ExtentsMapKeyCompare ( >> Extent = UserStruct; >> Block = (UINT32)(UINTN)StandaloneKey; >> >> - if (Block >= Extent->ee_block && Block < Extent->ee_block + >> Extent->ee_len) { >> + if (Block >= Extent->ee_block && Block < Extent->ee_block + >> Ext4GetExtentLength (Extent)) { >> return 0; >> } >> >> @@ -593,3 +593,23 @@ Ext4CheckExtentChecksum ( >> >> return Tail->eb_checksum == Ext4CalculateExtentChecksum (ExtHeader, >> File); >> } >> + >> +/** >> + Retrieves the extent's length, dealing with uninitialized extents in >> the process. >> + >> + @param[in] Extent Pointer to the EXT4_EXTENT >> + >> + @returns Extent's length, in filesystem blocks. >> +**/ >> +EXT4_BLOCK_NR >> +Ext4GetExtentLength ( >> + IN CONST EXT4_EXTENT *Extent >> + ) >> +{ >> + // If it's an unintialized extent, the true length is ee_len - 2^15 >> + if (EXT4_EXTENT_IS_UNINITIALIZED (Extent)) { >> + return Extent->ee_len - EXT4_EXTENT_MAX_INITIALIZED; >> + } >> + >> + return Extent->ee_len; >> +} >> diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c >> b/Features/Ext4Pkg/Ext4Dxe/Inode.c >> index 63cecec1f7..48bfe026a3 100644 >> --- a/Features/Ext4Pkg/Ext4Dxe/Inode.c >> +++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c >> @@ -144,11 +144,19 @@ Ext4Read ( >> >> HasBackingExtent = Status != EFI_NO_MAPPING; >> >> - if (!HasBackingExtent) { >> + if (!HasBackingExtent || EXT4_EXTENT_IS_UNINITIALIZED (&Extent)) { >> + >> HoleOff = BlockOff; >> - HoleLen = Partition->BlockSize - HoleOff; >> + >> + if (!HasBackingExtent) { >> + HoleLen = Partition->BlockSize - HoleOff; >> + } else { >> + // Uninitialized extents behave exactly the same as file holes. >> + HoleLen = Ext4GetExtentLength (&Extent) - HoleOff; >> + } >> + >> WasRead = HoleLen > RemainingRead ? RemainingRead : HoleLen; >> - // Potential improvement: In the future, we could get the hole's >> tota >> + // Potential improvement: In the future, we could get the file >> hole's total >> // size and memset all that >> SetMem (Buffer, WasRead, 0); >> } else { >> -- >> 2.33.1.windows.1 >> >> >> >> ------------ >> Groups.io Links: You receive all messages sent to this group. >> View/Reply Online (#82879): https://edk2.groups.io/g/devel/message/82879 >> Mute This Topic: https://groups.io/mt/86667035/5946980 >> Group Owner: devel+owner@edk2.groups.io >> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ >> pedro.falcato@gmail.com] >> ------------ >> >> >> > > -- > Pedro Falcato > -- Pedro Falcato