Pedro, I believe he DID reference Linux source
“ 2. I did't look at linux kernel(ext4) berfor send this patch, I cant found any offcial document, so I refer to linux source as a standand when send this patch” Kevin D Davis Security Strategist Insyde Software > On Oct 27, 2021, at 5:43 PM, qi zhou <atm...@outlook.com> wrote: > > This line may do come form linux kernel, As you can see in the first > link I refers says this number (1UL << 15) is kind of magic number. If > you write somethimg linux standanrded, It is hard to keep abosultely no > any linux involued > I think even freebsd has some code from linux, like the second link I > posted, the freebsd's ext4_ext_get_actual_len and EXT_INIT_MAX_LEN are > exactly the same as linux > > It is ok if it's considering as not mergeable, I think it is also good > just as a reference on the mailinng list, to those people who need to > read very large files > > The debug/fix process, I described here > > on the first, I use vbox's ext4 uefi driver to read large files, but > failed on verfication use some tools I writed, I share it here > md5sum.efi: https://1drv.ms/u/s!As-Ec5SPH0fuillwxhIsePY0KBla?e=WzHaBf > diff.efi: https://1drv.ms/u/s!As-Ec5SPH0fuilgMwlg6yNQOFCD1?e=GVoKuH > > then I googed to for replacement(on the first, I dont plat to fixed it > myself), But no luck, the all fails on large file read verfication. But > I noticed the performance of edk2-platforms's ext4 driver is most best > of all those uefi ext4 drivers > I did not found a working one, so I need to fix it. First I did some > guess and research, and then I added > some logs dump the edk2's read extents to serial on data that did't not > match, (the diff.efi tool I write will stop reading when data dismatch) > I compare those log dump to linux's 'filefrag -v"'s ouput, It is easy > to found the difference, then I google > to find the logic about unwritten extents, then did the fix > > > From: Pedro Falcato <pedro.falc...@gmail.com> > Sent: Thursday, October 28, 2021 5:34 > To: QiZhou <atm...@outlook.com> > Cc: devel@edk2.groups.io <devel@edk2.groups.io> > Subject: Re: [PATCH][Ext4Pkg] unwritten extent suuport > > Hi Qi, > > If you didn't use the Linux kernel (nor the documentation) as a reference, > can you please tell me what you've used? I'm asking because there's at least > a line that's suspiciously similar to Linux's code: > > #define EXTENT_INIT_MAX_LEN (1UL << 15) > > the UL looks redundant to me, since there's no need for it. > > Also, I prefer that you fix the typos yourself and format the patch > correctly, including the code. > > > On Wed, Oct 27, 2021 at 4:45 PM QiZhou <atm...@outlook.com> wrote: > 1. I am not familiar with freebsd, and don know if freebsd get the same issue, > But I do found the freebsd has some code snippets related to unwritten extent, > see: > https://github.com/freebsd/freebsd-src/blob/b3f46656393f5c8a6e8305afeb5e8c3638025c26/sys/fs/ext2fs/ext2_extents.h#L37 > https://github.com/freebsd/freebsd-src/blob/b3f46656393f5c8a6e8305afeb5e8c3638025c26/sys/fs/ext2fs/ext2_extents.c#L1347 > Is Ext4 is freebsd's default/major file system ? > > 2. I did't look at linux kernel(ext4) berfor send this patch, I cant > found any offcial document, so I refer to linux source as a standand > when send this patch > > 3. Yes, unwritten extents are wild used, usally when a file cotains many > zeros, or mark file holes(fallocate, qemu-img...) > You can generate a file contains a lot of unwritten extents by qemu, for > example: > qemu-img convert -f raw -O qcow2 win10.img win10.qcow2 > # win10.img's size: 10G > But for files do not have any continuous zeros, like compressed files, > then there will be no any unwritten extents > unwritten extents are usally seen in very large files > > 4. You can fix the typos, My English is not so good > >> On Oct 27 2021, at 10:56 pm, Pedro Falcato <pedro.falc...@gmail.com> wrote: >> >> 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 <atm...@outlook.com> wrote: >>> >>>> From: "Qi Zhou" <atm...@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/d25f27432f80a800a3592db128254c8140bd71bf/fs/ext4/ext4_extents.h#L156 >>>> >>>> Signed-off-by: Qi Zhou <atm...@outlook.com> >>>> --- >>>> 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 > > > -- > Pedro Falcato > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#82778): https://edk2.groups.io/g/devel/message/82778 Mute This Topic: https://groups.io/mt/86629612/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
smime.p7s
Description: S/MIME cryptographic signature
