From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"pedro.falcato@gmail.com" <pedro.falcato@gmail.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Subject: Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
Date: Fri, 19 Nov 2021 03:50:56 +0000 [thread overview]
Message-ID: <CO1PR11MB49291BAB16CF2FB3F07DE4E9D29C9@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CAKbZUD3qfS5pSvnFW-=8gv3Hp44+rgUfOUt4DkgAVddQkmQ1Vg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6603 bytes --]
Acked-by: Michael D Kinney michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Thursday, November 18, 2021 12:31 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Pedro Falcato <pedro.falcato@gmail.com>
Cc: Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
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 <pedro.falcato@gmail.com<mailto:pedro.falcato@gmail.com>> 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<http://groups.io> <pedro.falcato=gmail.com@groups.io<mailto:gmail.com@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 <leif@nuviainc.com<mailto:leif@nuviainc.com>>
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com<mailto:pedro.falcato@gmail.com>>
---
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<mailto:devel%2Bowner@edk2.groups.io>
Unsubscribe: https://edk2.groups.io/g/devel/unsub [pedro.falcato@gmail.com<mailto:pedro.falcato@gmail.com>]
------------
--
Pedro Falcato
--
Pedro Falcato
[-- Attachment #2: Type: text/html, Size: 48920 bytes --]
next prev parent reply other threads:[~2021-11-19 3:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <16B25AD607AC994B.20978@groups.io>
2021-11-01 21:29 ` [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support Pedro Falcato
2021-11-18 20:31 ` Pedro Falcato
2021-11-19 1:31 ` 回复: " gaoliming
2021-11-19 3:09 ` Pedro Falcato
2021-11-23 5:15 ` 回复: " gaoliming
2021-11-19 3:50 ` Michael D Kinney [this message]
2021-12-01 0:29 ` Pedro Falcato
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO1PR11MB49291BAB16CF2FB3F07DE4E9D29C9@CO1PR11MB4929.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox