Page MenuHomeFreeBSD

ext2fs: Add huge_file feature RW support
ClosedPublic

Authored by fsu on Jun 15 2017, 8:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 3, 11:03 PM
Unknown Object (File)
Fri, Oct 25, 7:31 AM
Unknown Object (File)
Oct 18 2024, 6:02 AM
Unknown Object (File)
Oct 17 2024, 3:06 AM
Unknown Object (File)
Oct 2 2024, 4:02 AM
Unknown Object (File)
Sep 17 2024, 4:10 PM
Unknown Object (File)
Sep 17 2024, 2:16 PM
Unknown Object (File)
Sep 7 2024, 8:12 AM
Subscribers

Details

Test Plan

Seems like, it is impossible to test it, because bigalloc feature does not supported by freebsd ext2 implementation.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/fs/ext2fs/ext2_inode_cnv.c
169 ↗(On Diff #29643)

Shouldn't those two lines be moved down, to a new "} else {" block?

fsu marked an inline comment as done.

Move normal case of i_blocks variable update inside if()

This is probably fine but I suspect this code has never been tested: a HUGE_FILE doesn't fit in a ext2 partition so the feature implies ext4. It also will take quite long to read/write so I would assume the filesystem will require some special implementation magic.

The code from ext2_inode_cnv.c could be removed until bigalloc will not be added. But in all cases EXT2F_ROCOMPAT_HUGE_FILE should be placed in correct place, because it is useful feature and we can not mount drives to RW mode.

In D11209#232007, @thisisadrgreenthumb_gmail.com wrote:

The code from ext2_inode_cnv.c could be removed until bigalloc will not be added. But in all cases EXT2F_ROCOMPAT_HUGE_FILE should be placed in correct place, because it is useful feature and we can not mount drives to RW mode.

To be honest .. the code doesn't bother where it is. At least i serves as a reminder tat we should implement that bigalloc thing.

I am not sure, that bigalloc could be implemented in nearest feature, because it is one of most complex ext4 features. Ok, I will update diff, where I will remove the if() expression ext2_i2ei(), but will leave EXT2F_ROCOMPAT_HUGE_FILE under EXT2F_ROCOMPAT_SUPP

Just add only feature to RO_COMPAT.

I think the flag is unnecessary in this case due to the way the feature flags work:

  • ROCOMPAT means that even if not supported we can mount the file system (which we do).
  • defining the feature would mean we support it fully, which would imply write support.

I prefer leaving it out as a reminder that this is one thing to do when/if we add write support.

In D11209#232380, @pfg wrote:

I think the flag is unnecessary in this case due to the way the feature flags work:

  • ROCOMPAT means that even if not supported we can mount the file system (which we do).
  • defining the feature would mean we support it fully, which would imply write support.

Unfortunately, we can mount it only in READ ONLY mode, but we need RW, because, this feature is usefull on relatively old linux distros.
We not need to add write support for this feature in this case, because files which are affected by it, are really big, as should be seen from my previous diff:

if (E2DI_HAS_HUGE_FILE(ip) && ip->i_blocks > 0xffffffffffffULL) // The max file sizes, depending of block sizes could be found here: https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout

I prefer leaving it out as a reminder that this is one thing to do when/if we add write support.

In D11209#232927, @thisisadrgreenthumb_gmail.com wrote:
In D11209#232380, @pfg wrote:

I think the flag is unnecessary in this case due to the way the feature flags work:

  • ROCOMPAT means that even if not supported we can mount the file system (which we do).
  • defining the feature would mean we support it fully, which would imply write support.

Unfortunately, we can mount it only in READ ONLY mode, but we need RW, because, this feature is usefull on relatively old linux distros.

You mean ext2/3 with enabled EXT2F_ROCOMPAT_HUGE_FILE? The case makes little sense but last time the feature flags work in such a way that we can still mount the filesystem RW: we will just not write huge files.

We not need to add write support for this feature in this case, because files which are affected by it, are really big, as should be seen from my previous diff:

if (E2DI_HAS_HUGE_FILE(ip) && ip->i_blocks > 0xffffffffffffULL) // The max file sizes, depending of block sizes could be found here: https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout

I have to look at it again but will a huge file fit in an ext2/3 filesystem? (sorry can't look that up right now)

In D11209#233019, @pfg wrote:
In D11209#232927, @thisisadrgreenthumb_gmail.com wrote:
In D11209#232380, @pfg wrote:

I think the flag is unnecessary in this case due to the way the feature flags work:

  • ROCOMPAT means that even if not supported we can mount the file system (which we do).
  • defining the feature would mean we support it fully, which would imply write support.

Unfortunately, we can mount it only in READ ONLY mode, but we need RW, because, this feature is usefull on relatively old linux distros.

You mean ext2/3 with enabled EXT2F_ROCOMPAT_HUGE_FILE? The case makes little sense but last time the feature flags work in such a way that we can still mount the filesystem RW: we will just not write huge files.

Using patch from https://reviews.freebsd.org/D11208:
root@user:~ # mkfs.ext2 -O huge_file /dev/md0
mke2fs 1.43.4 (31-Jan-2017)
/dev/md0 contains a ext4 file system
created on Sun Jun 18 22:48:36 2017
Proceed anyway? (y,N) y
Creating filesystem with 4096000 4k blocks and 1024000 inodes
Filesystem UUID: 204a9495-3f6a-44e0-b0df-b5b3c576ad4f
Superblock backups stored on blocks:
32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208

Allocating group tables: done
Writing inode tables: done
Writing superblocks and filesystem accounting information: done

root@user:~ # mount -t ext2fs /dev/md0 /mnt ; dmesg | tail -2
mount: /dev/md0: Invalid argument

WARNING: R/W mount of md0 denied due to unsupported optional features: huge_file

We not need to add write support for this feature in this case, because files which are affected by it, are really big, as should be seen from my previous diff:

if (E2DI_HAS_HUGE_FILE(ip) && ip->i_blocks > 0xffffffffffffULL) // The max file sizes, depending of block sizes could be found here: https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout

I have to look at it again but will a huge file fit in an ext2/3 filesystem? (sorry can't look that up right now)

OK, I'll bite this .. besides, it just can't hurt,

This revision is now accepted and ready to land.Jun 18 2017, 8:46 PM
This revision was automatically updated to reflect the committed changes.