From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) by mx.groups.io with SMTP id smtpd.web10.1717.1637267474945179034 for ; Thu, 18 Nov 2021 12:31:15 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=m0xbRW3f; spf=pass (domain: gmail.com, ip: 209.85.221.171, mailfrom: pedro.falcato@gmail.com) Received: by mail-vk1-f171.google.com with SMTP id k83so4574216vke.7 for ; Thu, 18 Nov 2021 12:31:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4XogsSsvCmr1RrRCiUcnG2yIAvINHwCHc6cc3Q2gX5A=; b=m0xbRW3fJtCYHT5dfASpXXmU1Okz4i6KNT/t8J2quu07dkkpaJxthWF4a4TNkDIILN u4lwIGnEoilaliDczh2XUc5PC9hvTJSlFkwF3DJ8Ug1X5f9VH0Wcf2j9ARD4eyrwuhWC JNssm6E0H8iyzcD5oKdVA4u1G7rIzhYhDwk5qnNPkIale/nir5DwlRRwVumZ8DP51SmT SPDlS094Mhv+z37ZQGVnEEWgKsNP8e2FyzuZpQchvrw/QaUaTd1kVp5SN+XRyhzzQxas +5cE116kT6LPkSxvUwRLchjpjyZuTYVDgd8ySjMCckkWSd+mhtkGzXz2EzFDzuv7oP94 +giw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4XogsSsvCmr1RrRCiUcnG2yIAvINHwCHc6cc3Q2gX5A=; b=R/cmr4TOpQl/lq0uUS655CpQTGDo67E6/bcTjW8GzzPOyzORc4x2OA7X7xbRI0hctB ei7M6A0CL6qRBCVSWz9x6WI0Kc65Wf+mwRcSnTYCX5ZI20EWV/LED/PeH3R+vWF06jLZ nNoMzu0eByIH/78C8W6Iy3xq6ZhoS/Tx6Kr7OQ96tSMxvrCmkMUBy6wOhdgHyEeP1UpK sVoJFRC6cj7AMaSHn01/rI7lOKPxYI+3PHtjYBMqk5De+jQLJt4myRxHL0xkrtJxlDlx rlP35AB75A/OGg60/eq8o1QYisiELYpS3cX3I8+8DC6hg80W1wuAEF8gbjPbZVT3crEg YR4A== X-Gm-Message-State: AOAM533DosRxUYDlcNSCJ7L9Pkfu0RIiYWM8j7gZCatGLjhlHJqpdEcC 58dbOQBBAXDAfLG6pJqwC0FLCPxNGPFnc1q+x3/pkNsq3EE= X-Google-Smtp-Source: ABdhPJz8JT7IjJ0ZKwtvLq9qqPPzqjUGLVvx+KY+cU87V7mr+XUPSmempnJlSWy0kGAcjZ5taUNzFHpWj8/EYjdKoug= X-Received: by 2002:a05:6122:c98:: with SMTP id ba24mr107279240vkb.25.1637267473867; Thu, 18 Nov 2021 12:31:13 -0800 (PST) MIME-Version: 1.0 References: <16B25AD607AC994B.20978@groups.io> In-Reply-To: From: "Pedro Falcato" Date: Thu, 18 Nov 2021 20:31:02 +0000 Message-ID: Subject: Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support To: edk2-devel-groups-io , Pedro Falcato Cc: Leif Lindholm , Michael D Kinney Content-Type: multipart/alternative; boundary="0000000000002ea84105d1160a46" --0000000000002ea84105d1160a46 Content-Type: text/plain; charset="UTF-8" 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 --0000000000002ea84105d1160a46 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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 <ped= ro.falcato@gmail.com> wrote:
Quick PS: I CC'd the edk2-platform= s 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, p= lease let me know.

Thanks,
Pedro
On Fri, = Oct 29, 2021 at 2:04 AM Pedro Falcato via groups.io <pedro.falcato=3Dgmail.com@groups.io> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex">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 <leif@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
---
=C2=A0Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h |=C2=A0 8 ++++++++
=C2=A0Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h=C2=A0 | 21 +++++++++++++++++++++ =C2=A0Features/Ext4Pkg/Ext4Dxe/Extents.c=C2=A0 | 24 ++++++++++++++++++++++-= -
=C2=A0Features/Ext4Pkg/Ext4Dxe/Inode.c=C2=A0 =C2=A0 | 14 +++++++++++---
=C2=A04 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 {
=C2=A0 =C2=A0UINT32=C2=A0 =C2=A0 eb_checksum;
=C2=A0} 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 un= initialized.
+ * Long story short, it's an extent that behaves as a file hole but ha= s blocks already allocated.
+ */
+#define EXT4_EXTENT_MAX_INITIALIZED=C2=A0 (1 << 15)
+
=C2=A0typedef UINT64=C2=A0 EXT4_BLOCK_NR;
=C2=A0typedef UINT32=C2=A0 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 (
=C2=A0 =C2=A0IN EFI_BLOCK_IO_PROTOCOL=C2=A0 *BlockIo
=C2=A0 =C2=A0);

+/**
+=C2=A0 =C2=A0Check if the extent is uninitialized
+
+=C2=A0 =C2=A0@param[in] Extent=C2=A0 =C2=A0 Pointer to the EXT4_EXTENT
+
+=C2=A0 =C2=A0@returns True if uninitialized, else false.
+**/
+#define EXT4_EXTENT_IS_UNINITIALIZED(Extent) ((Extent)->ee_len > EXT= 4_EXTENT_MAX_INITIALIZED)
+
+/**
+=C2=A0 =C2=A0Retrieves the extent's length, dealing with uninitialized= extents in the process.
+
+=C2=A0 =C2=A0@param[in] Extent=C2=A0 =C2=A0 =C2=A0 Pointer to the EXT4_EXT= ENT
+
+=C2=A0 =C2=A0@returns Extent's length, in filesystem blocks.
+**/
+EXT4_BLOCK_NR
+Ext4GetExtentLength (
+=C2=A0 IN CONST EXT4_EXTENT=C2=A0 *Extent
+=C2=A0 );
+
=C2=A0#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 (
=C2=A0 =C2=A0 =C2=A0return EFI_NO_MAPPING;
=C2=A0 =C2=A0}

-=C2=A0 if (!(LogicalBlock >=3D Ext->ee_block && Ext->ee_b= lock + Ext->ee_len > LogicalBlock)) {
+=C2=A0 if (!(LogicalBlock >=3D Ext->ee_block && Ext->ee_b= lock + Ext4GetExtentLength (Ext) > LogicalBlock)) {
=C2=A0 =C2=A0 =C2=A0// This extent does not cover the block
=C2=A0 =C2=A0 =C2=A0if (Buffer !=3D NULL) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0FreePool (Buffer);
@@ -413,7 +413,7 @@ Ext4ExtentsMapKeyCompare (
=C2=A0 =C2=A0Extent =3D UserStruct;
=C2=A0 =C2=A0Block=C2=A0 =3D (UINT32)(UINTN)StandaloneKey;

-=C2=A0 if (Block >=3D Extent->ee_block && Block < Extent-= >ee_block + Extent->ee_len) {
+=C2=A0 if (Block >=3D Extent->ee_block && Block < Extent-= >ee_block + Ext4GetExtentLength (Extent)) {
=C2=A0 =C2=A0 =C2=A0return 0;
=C2=A0 =C2=A0}

@@ -593,3 +593,23 @@ Ext4CheckExtentChecksum (

=C2=A0 =C2=A0return Tail->eb_checksum =3D=3D Ext4CalculateExtentChecksum= (ExtHeader, File);
=C2=A0}
+
+/**
+=C2=A0 =C2=A0Retrieves the extent's length, dealing with uninitialized= extents in the process.
+
+=C2=A0 =C2=A0@param[in] Extent=C2=A0 =C2=A0 =C2=A0 Pointer to the EXT4_EXT= ENT
+
+=C2=A0 =C2=A0@returns Extent's length, in filesystem blocks.
+**/
+EXT4_BLOCK_NR
+Ext4GetExtentLength (
+=C2=A0 IN CONST EXT4_EXTENT=C2=A0 *Extent
+=C2=A0 )
+{
+=C2=A0 // If it's an unintialized extent, the true length is ee_len - = 2^15
+=C2=A0 if (EXT4_EXTENT_IS_UNINITIALIZED (Extent)) {
+=C2=A0 =C2=A0 return Extent->ee_len - EXT4_EXTENT_MAX_INITIALIZED;
+=C2=A0 }
+
+=C2=A0 return Extent->ee_len;
+}
diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c b/Features/Ext4Pkg/Ext4Dxe/In= ode.c
index 63cecec1f7..48bfe026a3 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
@@ -144,11 +144,19 @@ Ext4Read (

=C2=A0 =C2=A0 =C2=A0HasBackingExtent =3D Status !=3D EFI_NO_MAPPING;

-=C2=A0 =C2=A0 if (!HasBackingExtent) {
+=C2=A0 =C2=A0 if (!HasBackingExtent || EXT4_EXTENT_IS_UNINITIALIZED (&= Extent)) {
+
=C2=A0 =C2=A0 =C2=A0 =C2=A0HoleOff =3D BlockOff;
-=C2=A0 =C2=A0 =C2=A0 HoleLen =3D Partition->BlockSize - HoleOff;
+
+=C2=A0 =C2=A0 =C2=A0 if (!HasBackingExtent) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 HoleLen =3D Partition->BlockSize - HoleOff;=
+=C2=A0 =C2=A0 =C2=A0 } else {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 // Uninitialized extents behave exactly the sa= me as file holes.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 HoleLen =3D Ext4GetExtentLength (&Extent) = - HoleOff;
+=C2=A0 =C2=A0 =C2=A0 }
+
=C2=A0 =C2=A0 =C2=A0 =C2=A0WasRead =3D HoleLen > RemainingRead ? Remaini= ngRead : HoleLen;
-=C2=A0 =C2=A0 =C2=A0 // Potential improvement: In the future, we could get= the hole's tota
+=C2=A0 =C2=A0 =C2=A0 // Potential improvement: In the future, we could get= the file hole's total
=C2=A0 =C2=A0 =C2=A0 =C2=A0// size and memset all that
=C2=A0 =C2=A0 =C2=A0 =C2=A0SetMem (Buffer, WasRead, 0);
=C2=A0 =C2=A0 =C2=A0} 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/dev= el/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.c= om]
------------




--
Pedro Falcato


--
Pedro Falcato
--0000000000002ea84105d1160a46--