From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ua1-f43.google.com (mail-ua1-f43.google.com [209.85.222.43]) by mx.groups.io with SMTP id smtpd.web11.12851.1635346576384494862 for ; Wed, 27 Oct 2021 07:56:16 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=Re70i7HH; spf=pass (domain: gmail.com, ip: 209.85.222.43, mailfrom: pedro.falcato@gmail.com) Received: by mail-ua1-f43.google.com with SMTP id v3so5413757uam.10 for ; Wed, 27 Oct 2021 07:56:16 -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=Gl40UB/CMDrit2VYSJSlVoQNTHtUVWxILxQzXqLxvtA=; b=Re70i7HHAjBKN9Excua+LsXtMLmzrQRXHiAqK0vjZun2icnuf2XOAwsovPsNhP6AJX 4hpPWAFTJpbvXmsTNxj4YDBzV+BrIPCDaMbq8gNivm9wpbn9smfFCxCITmjzG0yez0xQ mJU5PmcUdiWHQZVOWhbCnkBiQ7+ZBHSKP/An8tq7hewDC9/urUbCN7HZGQK/Z2aLIbhR nqmRG4VevkugsMp4uhR75pWTdMmDeiyNXkVnE2RESrgizHeyEULyMPxub5zCi/xAkl6O 8uBPH6IHrOQidUi4ObXzQPVU54jtVnthj+wivvEEILPhMyTwTAZorWiIcsyzo8BgvaAr FfOQ== 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=Gl40UB/CMDrit2VYSJSlVoQNTHtUVWxILxQzXqLxvtA=; b=SmxDSLn3Pe+mHRAlBUfxEkKevOA+/CQLkcZii1D2/1nOH+6riBhhm2gJkLuWao3mko Ww0iZWKo5VHFCkDVNhjhWYCFX6PV0zugUbZZrDihA1+i5Kk+0Tlc3SlGHndFZLj+h+vA yjD+kQNwrUP6neTWcI90DUPDNbKBdDM/VJAuvNnUQtlD9JIfhG6+5qyaFUjOMg3JmmN1 4LWrLY0dHyFtYqbqx/rAbascxu6XrO11dDYIMq304g82YJjmWHqsZKGBdE7PWdCi1EbT tOaKn8xRhbtUq5B25L9gRYL0E1WNSCn6XAz0X6RWrKppWNp20vYHeeQ/htIL7/7pqQOT 72nQ== X-Gm-Message-State: AOAM532MXWkQRnOy/IvD0snwjnWeVj7y4yoRFrHbgCeIbO0i7/YkkcVy p0I9Na5jKNkusRxAntQ4XL+8gCKDSc4dUcpKqBOXGTlpDY8Wtw== X-Google-Smtp-Source: ABdhPJxQfByGxse7j2WRaNw4W3ZWM1IpNk2FDwKiTI9DI35+xa8yb5p7L/pb6jROzvFZ7b6fv8OFak0CoTzMviP/dL4= X-Received: by 2002:ab0:455:: with SMTP id 79mr14833200uav.103.1635346575499; Wed, 27 Oct 2021 07:56:15 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "Pedro Falcato" Date: Wed, 27 Oct 2021 15:56:02 +0100 Message-ID: Subject: Re: [PATCH][Ext4Pkg] unwritten extent suuport To: qi zhou Cc: "devel@edk2.groups.io" Content-Type: multipart/alternative; boundary="000000000000b7a5db05cf56cbed" --000000000000b7a5db05cf56cbed Content-Type: text/plain; charset="UTF-8" Hi, The patch looks OK despite the typos and lack of proper formatting on the commit message. But honestly, I don't know if this patch is even mergeable considering you looked at the Linux kernel's source code for this. The patch was already trivial enough if you looked at the documentation and the FreeBSD driver (as I had done in the past but never got to fixing this considering I don't even know if unwritten extents can appear in the wild). I *cannot* stress this enough: Ext4Pkg is a clean room implementation of ext4 licensed under the BSD-2-Clause-Patent license (which is NOT compatible with GPLv2) and cannot have random Linux kernel bits, or any other incompatibly-licensed project's bits for that matter. Best regards, Pedro On Wed, Oct 27, 2021 at 2:37 PM qi zhou wrote: > From: "Qi Zhou" > Subject: [PATCH] unwritten extent suuport > > the real lenght of uninitialized/unwritten extent should be (ee_len - (1UL > << 15)), and > all related block should been read as zeros. see: > > https://github.com/torvalds/linux/blob/d25f27432f80a800a3592db128254c8140bd71bf/fs/ext4/ext4_extents.h#L156 > > Signed-off-by: Qi Zhou > --- > Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 5 +++++ > Features/Ext4Pkg/Ext4Dxe/Extents.c | 4 ++-- > Features/Ext4Pkg/Ext4Dxe/Inode.c | 5 +++++ > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > index 070eb5a..7ca8eee 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > @@ -402,6 +402,11 @@ typedef struct { > > #define EXT4_MIN_DIR_ENTRY_LEN 8 > > +#define EXTENT_INIT_MAX_LEN (1UL << 15) > + > +#define EXTENT_REAL_LEN(x) ((UINT16)(x <= EXTENT_INIT_MAX_LEN ? x : (x - > EXTENT_INIT_MAX_LEN))) > +#define EXTENT_IS_UNWRITTEN(x) (x > EXTENT_INIT_MAX_LEN) > + > // This on-disk structure is present at the bottom of the extent tree > typedef struct { > // First logical block > diff --git a/Features/Ext4Pkg/Ext4Dxe/Extents.c > b/Features/Ext4Pkg/Ext4Dxe/Extents.c > index 5fa2fe0..21af573 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 + > EXTENT_REAL_LEN(Ext->ee_len) > 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 + > EXTENT_REAL_LEN(Extent->ee_len)) { > return 0; > } > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c > b/Features/Ext4Pkg/Ext4Dxe/Inode.c > index 63cecec..d691ec7 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Inode.c > +++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c > @@ -151,6 +151,11 @@ Ext4Read ( > // Potential improvement: In the future, we could get the hole's > tota > // size and memset all that > SetMem (Buffer, WasRead, 0); > + } else if(EXTENT_IS_UNWRITTEN(Extent.ee_len)) { > + HoleOff = CurrentSeek - (UINT64)Extent.ee_block * > Partition->BlockSize; > + HoleLen = EXTENT_REAL_LEN(Extent.ee_len) * Partition->BlockSize - > HoleOff; > + WasRead = HoleLen > RemainingRead ? RemainingRead : HoleLen; > + SetMem (Buffer, WasRead, 0); > } else { > ExtentStartBytes = MultU64x32 ( > LShiftU64 (Extent.ee_start_hi, 32) | > -- > 2.17.1 > > -- Pedro Falcato --000000000000b7a5db05cf56cbed Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,

The patch looks OK despi= te the typos and lack of proper formatting on the commit message.

But honestly, I don't know if this patch is even mergea= ble considering you looked at the Linux kernel's source code for this. = The patch was already trivial enough
if you looked at the documen= tation and the FreeBSD driver (as I had done in the past but never got to f= ixing this considering I don't even know if unwritten extents can appea= r in the wild).

I *cannot* stress this enough: Ext= 4Pkg is a clean room implementation of ext4 licensed under the BSD-2-Clause-Patent license (which is NOT compatible with G= PLv2) and cannot have random Linux kernel
bits, or any other incompatibly-licensed project's bits for = that matter.

Best regards,
Pedro

On Wed, Oct 27, 2021 at 2:37 PM qi = zhou <atmgnd@outlook.com> w= rote:
From: &quo= t;Qi Zhou" <atmgnd@outlook.com>
Subject: [PATCH] unwritten extent suuport

the real lenght of uninitialized/unwritten extent should be (ee_len - (1UL = << 15)), and
all related block should been read as zeros. see:
https://github.com/torvalds/linux/blob/d25f27432f80a800a3592db128254= c8140bd71bf/fs/ext4/ext4_extents.h#L156

Signed-off-by: Qi Zhou <atmgnd@outlook.com>
---
=C2=A0Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 5 +++++
=C2=A0Features/Ext4Pkg/Ext4Dxe/Extents.c=C2=A0 | 4 ++--
=C2=A0Features/Ext4Pkg/Ext4Dxe/Inode.c=C2=A0 =C2=A0 | 5 +++++
=C2=A03 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h b/Features/Ext4Pkg/Ext4Dxe= /Ext4Disk.h
index 070eb5a..7ca8eee 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
@@ -402,6 +402,11 @@ typedef struct {

=C2=A0#define EXT4_MIN_DIR_ENTRY_LEN=C2=A0 8

+#define EXTENT_INIT_MAX_LEN (1UL << 15)
+
+#define EXTENT_REAL_LEN(x) ((UINT16)(x <=3D EXTENT_INIT_MAX_LEN ? x : (= x - EXTENT_INIT_MAX_LEN)))
+#define EXTENT_IS_UNWRITTEN(x) (x > EXTENT_INIT_MAX_LEN)
+
=C2=A0// This on-disk structure is present at the bottom of the extent tree=
=C2=A0typedef struct {
=C2=A0 =C2=A0// First logical block
diff --git a/Features/Ext4Pkg/Ext4Dxe/Extents.c b/Features/Ext4Pkg/Ext4Dxe/= Extents.c
index 5fa2fe0..21af573 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 + EXTENT_REAL_LEN(Ext->ee_len) > 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 + EXTENT_REAL_LEN(Extent->ee_len)) {
=C2=A0 =C2=A0 =C2=A0return 0;
=C2=A0 =C2=A0}

diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c b/Features/Ext4Pkg/Ext4Dxe/In= ode.c
index 63cecec..d691ec7 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
@@ -151,6 +151,11 @@ Ext4Read (
=C2=A0 =C2=A0 =C2=A0 =C2=A0// Potential improvement: In the future, we coul= d get the hole's tota
=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 } else if(EXTENT_IS_UNWRITTEN(Extent.ee_len)) {
+=C2=A0 =C2=A0 =C2=A0 HoleOff =3D CurrentSeek - (UINT64)Extent.ee_block * P= artition->BlockSize;
+=C2=A0 =C2=A0 =C2=A0 HoleLen =3D EXTENT_REAL_LEN(Extent.ee_len) * Partitio= n->BlockSize - HoleOff;
+=C2=A0 =C2=A0 =C2=A0 WasRead =3D HoleLen > RemainingRead ? RemainingRea= d : HoleLen;
+=C2=A0 =C2=A0 =C2=A0 SetMem (Buffer, WasRead, 0);
=C2=A0 =C2=A0 =C2=A0} else {
=C2=A0 =C2=A0 =C2=A0 =C2=A0ExtentStartBytes =3D MultU64x32 (
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 LShiftU64 (Extent.ee_start_hi, 32) |
--
2.17.1



--
Pedro Falcato
--000000000000b7a5db05cf56cbed--