From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ua1-f47.google.com (mail-ua1-f47.google.com [209.85.222.47]) by mx.groups.io with SMTP id smtpd.web10.850.1635802205000901203 for ; Mon, 01 Nov 2021 14:30:05 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=SUCmG8w0; spf=pass (domain: gmail.com, ip: 209.85.222.47, mailfrom: pedro.falcato@gmail.com) Received: by mail-ua1-f47.google.com with SMTP id v20so34283903uaj.9 for ; Mon, 01 Nov 2021 14:30:04 -0700 (PDT) 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=mTGRlL0c9aCSbWMKPMrwFd81cVe1tT9FX5833J8i9w0=; b=SUCmG8w0oadyNYoCMHi6/8exyjW3sdZu1QJ39/rmzMiviLIOdeERw1KOsUhrhagBdG M2mto+ws3Cb8LvURh8QBv2n7uvg8WI1Pej+V9MZ1TbZW/bHX/TAtLvLHZ73L/rbXKIvX 87mA6rwFzaCDwS6DMuDL2z5O1eoJK4oiduDtYgU1xcProfODP2MMI8Y7YIOqTMVal8hv Y/pZS+BJ+r6jRuMgDCgcmyB50c343T4aq0p1pbCwpSVylyT4KL6JIYxrluQFivEqeaRj y/uN1Wb27ypgHkeW8v6jY3ez1kyQ6GiublQ60amQSKGjuJW8sxzkfQkItJE8iGd0kZ25 0MLw== 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=mTGRlL0c9aCSbWMKPMrwFd81cVe1tT9FX5833J8i9w0=; b=0AvURHAT9FphwuIPpjuGmvrmlH6RAH58PMbZBT3suT6YiL+YRZsVASJ7jh6fFaXpYd nSSpXvdWiqDSzNbyBMnbv1sx3NtB4Vn5FB6k1PXyfe2NMyIJN26RsxZFYk1Ux7U8frWp MbsQUAxfmMhuyrBNG4LqtS0xslpSdqQNYTJ91dWamI7qD3Gg3MFQZtF6w+zI+FdNYbwL kUZr2mUWlw26Tx/Mwb8Cvx759R4LvshJvPlIzJ3tpXfqqdUV1E2hkYKMfvlyhj4l10mA SNFtPGAQMGoykyiMCvJfRGBSTO/6QQwaYryqdHCHHOeZu2oakcSmpaJ4ADyrighphtAy j+RA== X-Gm-Message-State: AOAM532aS9Cb+p/qUMKfkWRKJoN8pQ0Yo9Ss0lRH3yBT2jbbK2q+ae12 NFMtE7Dzfd9WsfPeJbbLMh/gdeHqf7RWGu4RNdBSeC7nEo83TQ== X-Google-Smtp-Source: ABdhPJw2nICUn3N9o3HdGF4sl9wwilml+3dvsIaeZr60xqdLoGjF1vfrFZRE0zkj7cllhOGV2glpLthMc9QbAqk5Mng= X-Received: by 2002:a05:6130:30a:: with SMTP id ay10mr22261757uab.135.1635802203990; Mon, 01 Nov 2021 14:30:03 -0700 (PDT) MIME-Version: 1.0 References: <16B25AD607AC994B.20978@groups.io> In-Reply-To: <16B25AD607AC994B.20978@groups.io> From: "Pedro Falcato" Date: Mon, 1 Nov 2021 21:29:52 +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="0000000000004aaf5b05cfc0e10f" --0000000000004aaf5b05cfc0e10f Content-Type: text/plain; charset="UTF-8" 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 --0000000000004aaf5b05cfc0e10f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Quick PS: I CC'd the edk2-platforms stewards on the pa= tch because there's no co-maintainer, and from what I gathered, that= 9;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 <pedro.= falcato=3Dgmail.com@groups.io>= ; wrote:
Uniniti= alized 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
--0000000000004aaf5b05cfc0e10f--