public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
       [not found] <16B25AD607AC994B.20978@groups.io>
@ 2021-11-01 21:29 ` Pedro Falcato
  2021-11-18 20:31   ` Pedro Falcato
  2021-12-01  0:29 ` Pedro Falcato
  1 sibling, 1 reply; 7+ messages in thread
From: Pedro Falcato @ 2021-11-01 21:29 UTC (permalink / raw)
  To: edk2-devel-groups-io, Pedro Falcato; +Cc: Leif Lindholm, Michael D Kinney

[-- Attachment #1: Type: text/plain, Size: 5738 bytes --]

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 <pedro.falcato=
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>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Pedro Falcato <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
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [pedro.falcato@gmail.com
> ]
> ------------
>
>
>

-- 
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 7532 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
  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:50     ` Michael D Kinney
  0 siblings, 2 replies; 7+ messages in thread
From: Pedro Falcato @ 2021-11-18 20:31 UTC (permalink / raw)
  To: edk2-devel-groups-io, Pedro Falcato; +Cc: Leif Lindholm, Michael D Kinney

[-- Attachment #1: Type: text/plain, Size: 6219 bytes --]

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>
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
> <pedro.falcato=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>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Signed-off-by: Pedro Falcato <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
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [
>> pedro.falcato@gmail.com]
>> ------------
>>
>>
>>
>
> --
> Pedro Falcato
>


-- 
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 8271 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* 回复: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
  2021-11-18 20:31   ` Pedro Falcato
@ 2021-11-19  1:31     ` gaoliming
  2021-11-19  3:09       ` Pedro Falcato
  2021-11-19  3:50     ` Michael D Kinney
  1 sibling, 1 reply; 7+ messages in thread
From: gaoliming @ 2021-11-19  1:31 UTC (permalink / raw)
  To: devel, pedro.falcato; +Cc: 'Leif Lindholm', 'Michael D Kinney'

[-- Attachment #1: Type: text/plain, Size: 6837 bytes --]

Pedro:

 I review this patch. This change looks good. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

 

 Besides, I want to confirm whether Ext4Dxe support Ext2/Ext3/Ext4 file system or only Ext4 file system.

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Pedro Falcato
发送时间: 2021年11月19日 4:31
收件人: edk2-devel-groups-io <devel@edk2.groups.io>; Pedro Falcato <pedro.falcato@gmail.com>
抄送: Leif Lindholm <leif@nuviainc.com>; Michael D Kinney <michael.d.kinney@intel.com>
主题: 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: 13722 bytes --]

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
  2021-11-19  1:31     ` 回复: " gaoliming
@ 2021-11-19  3:09       ` Pedro Falcato
  2021-11-23  5:15         ` 回复: " gaoliming
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Falcato @ 2021-11-19  3:09 UTC (permalink / raw)
  To: edk2-devel-groups-io, Liming Gao

[-- Attachment #1: Type: text/plain, Size: 7525 bytes --]

Hi Liming,

Thanks for the RB :) I'll be pushing this soon.

Ext4Dxe at the moment only supports ext4 due to a lack of need for ext2/3
(since ext4 itself is already decently old), but with a bit of work it can
support all the previous ext's. If you have a need for it, feel free to let
me know and file a bugzilla, and I'll take a stab at it in the future.

Best regards,
Pedro

On Fri, Nov 19, 2021 at 1:31 AM gaoliming <gaoliming@byosoft.com.cn> wrote:

> Pedro:
>
>  I review this patch. This change looks good. Reviewed-by: Liming Gao <
> gaoliming@byosoft.com.cn>
>
>
>
>  Besides, I want to confirm whether Ext4Dxe support Ext2/Ext3/Ext4 file
> system or only Ext4 file system.
>
>
>
> Thanks
>
> Liming
>
> *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Pedro Falcato
> *发送时间:* 2021年11月19日 4:31
> *收件人:* edk2-devel-groups-io <devel@edk2.groups.io>; Pedro Falcato <
> pedro.falcato@gmail.com>
> *抄送:* Leif Lindholm <leif@nuviainc.com>; Michael D Kinney <
> michael.d.kinney@intel.com>
> *主题:* 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>
> 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
> <pedro.falcato=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>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Pedro Falcato <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
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [pedro.falcato@gmail.com
> ]
> ------------
>
>
>
>
> --
>
> Pedro Falcato
>
>
>
> --
>
> Pedro Falcato
>
> 
>
>

-- 
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 13317 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
  2021-11-18 20:31   ` Pedro Falcato
  2021-11-19  1:31     ` 回复: " gaoliming
@ 2021-11-19  3:50     ` Michael D Kinney
  1 sibling, 0 replies; 7+ messages in thread
From: Michael D Kinney @ 2021-11-19  3:50 UTC (permalink / raw)
  To: devel@edk2.groups.io, pedro.falcato@gmail.com, Kinney, Michael D
  Cc: Leif Lindholm

[-- 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 --]

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* 回复: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
  2021-11-19  3:09       ` Pedro Falcato
@ 2021-11-23  5:15         ` gaoliming
  0 siblings, 0 replies; 7+ messages in thread
From: gaoliming @ 2021-11-23  5:15 UTC (permalink / raw)
  To: devel, pedro.falcato

[-- Attachment #1: Type: text/plain, Size: 8059 bytes --]

Pedro:

 I submit BZ https://bugzilla.tianocore.org/show_bug.cgi?id=3745 to request Ext2/Ext3 file system.

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Pedro Falcato
发送时间: 2021年11月19日 11:10
收件人: edk2-devel-groups-io <devel@edk2.groups.io>; Liming Gao <gaoliming@byosoft.com.cn>
主题: Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support

 

Hi Liming,

 

Thanks for the RB :) I'll be pushing this soon.

 

Ext4Dxe at the moment only supports ext4 due to a lack of need for ext2/3 (since ext4 itself is already decently old), but with a bit of work it can support all the previous ext's. If you have a need for it, feel free to let me know and file a bugzilla, and I'll take a stab at it in the future.

 

Best regards,

Pedro

 

On Fri, Nov 19, 2021 at 1:31 AM gaoliming <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> > wrote:

Pedro:

 I review this patch. This change looks good. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >

 

 Besides, I want to confirm whether Ext4Dxe support Ext2/Ext3/Ext4 file system or only Ext4 file system.

 

Thanks

Liming

发件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io>  <devel@edk2.groups.io <mailto:devel@edk2.groups.io> > 代表 Pedro Falcato
发送时间: 2021年11月19日 4:31
收件人: edk2-devel-groups-io <devel@edk2.groups.io <mailto:devel@edk2.groups.io> >; Pedro Falcato <pedro.falcato@gmail.com <mailto:pedro.falcato@gmail.com> >
抄送: Leif Lindholm <leif@nuviainc.com <mailto:leif@nuviainc.com> >; Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> >
主题: 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



-- 

Pedro Falcato




[-- Attachment #2: Type: text/html, Size: 19728 bytes --]

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support
       [not found] <16B25AD607AC994B.20978@groups.io>
  2021-11-01 21:29 ` [edk2-devel] [edk2-platforms PATCH] Ext4Pkg: Add uninitialized extents support Pedro Falcato
@ 2021-12-01  0:29 ` Pedro Falcato
  1 sibling, 0 replies; 7+ messages in thread
From: Pedro Falcato @ 2021-12-01  0:29 UTC (permalink / raw)
  To: edk2-devel-groups-io, Pedro Falcato

[-- Attachment #1: Type: text/plain, Size: 5606 bytes --]

Pushed to edk2-platforms as 2c6da5c2cb5b3bd13aad5b6dd535a241c83e4b8a.

On Fri, Oct 29, 2021 at 2:04 AM Pedro Falcato via groups.io <pedro.falcato=
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>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Pedro Falcato <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
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [pedro.falcato@gmail.com
> ]
> ------------
>
>
>

-- 
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 7346 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-12-01  0:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [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
2021-12-01  0:29 ` Pedro Falcato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox