Hi,
few comments
1) the patch needs to be split into 2, ext2 change/lzo change
2) send them inline please and not as attachement
3) the V3 needs to be in the subject prefix [PATCH V3] procd: ...
4) the Signed-off-by: line is missing
John
On 03/07/2016 08:14, Luke McKee wrote:
> To quote Arjen:
> If the only reason to switch to ext2 is to remove the journal, why not
> just add
>
> -O ^has_journal
>
> to the mount options?
>
> ----
>
> That's not a mount option. That's a tune2fs option.
>
> Journaling isn't the the only problem. The biggest problem is BLOAT.
> You need libext2+e2fsprogs in a 4MB flash root image with the current
> zram setup.
> Would someone want /tmp on zram if they had to add all this to the 4MB
> squashfs image.
>
> -rw-r--r-- 1 lmc users 199739 Jul 3 06:15 e2fsprogs_1.43.1-1_mips_24kc.ipk
> -rw-r--r-- 1 lmc users 151873 Jul 3 06:15 libext2fs_1.43.1-1_mips_24kc.ipk
>
> The simple answer is most openwrt users would say no to this feature
> do to how it's implemented.
>
> I actually was going for ext2 like the original patch, but I was sold
> that ext4 can mount ext2 formatted filesystems (without a journal)
> (see man ext4). The ext4 filesystem driver vs ext2 does bloat a bit,
> but I don't mind it as I can mount ext4 over nbd for my extroot.
>
> Busybox is only 40k of source including the headers. mkfs.ext4 is 360k
> compressed!
> https://git.busybox.net/busybox/tree/util-linux/mkfs_ext2.c
>
> Also -O ^has_journal is an tune2fs option not a mount option. And that
> tune2fs feature isn't in busybox, hence the bloat issue.
>
> "BusyBox v1.24.2 () multi-call binary.
>
> Usage: tune2fs [-c MAX_MOUNT_COUNT] [-i DAYS] [-C MOUNT_COUNT] [-L
> LABEL] BLOCKDEV
>
> Adjust filesystem options on ext[23] filesystems"
> Busybox tune2fs is useless.
>
> So 90% of the userbase with size constrained flash file-systems will
> want to use busybox mkfs.ext2 instead. busybox mkfs.ext2 and mkfs.ext2
> don't conflict because the package installs in /usr/sbin and busybox
> installs in /sbin
>
> I move that the feature be dependent ONLY on busybox mkfs.ext2.
> Busybox mount isn't required because procd includes sys/mount.h
> In order words set the path to /sbin/mkfs.ext2 (busybox)
>
> Attached is the new patch. Allow users in the Makefile configuration
> to choose to use LZ4 (more speed ~15% less compression) if they so
> desire.
> John can do the makefile if and when he merges the patch.
>
> I'm out of the debate now, I got it in my
> lede/packgaes/system/procd/patches/ directory :) I'm happy
>
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
