ext2fs: Add uninit_bg feature support.
ClosedPublic
Actions

Authored by fsu on Jun 15 2017, 8:21 AM.

Details

Reviewers

pfg
kevlo
trasz
rwatson

Commits

rS320145: ext2fs: Add uninit_bg feature support.

Test Plan

The next script{F1418349} and fstorture from: https://github.com/thedrgreenthumb/fstools

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

fsu created this revision.Jun 15 2017, 8:21 AM

Herald added a subscriber: imp. · View Herald TranscriptJun 15 2017, 8:21 AM

I don't know much about this feature but it is probably interesting enough to put all the new functionality in a different file.
ext2_alloc is already crowded.

Ok, you are right, I will try to add file like ext2_csum.c

In D11211#232008, @thisisadrgreenthumb_gmail.com wrote:

Ok, you are right, I will try to add file like ext2_csum.c

BTW, I think we might have code for CRCs in the kernel already. Not sure but ext2_crc16() may be redundant,

Move csum logic to own file.

BTW, I think we might have code for CRCs in the kernel already. Not sure but ext2_crc16() may be redundant,

Seems like, it is impossible. Because only one crc16 implementation in kernel exist, and it is incompatible with ext2 crc16.

Looks good to me.

sys/fs/ext2fs/ext2_alloc.c
755 ↗	(On Diff #29709)	We have some bitmap manipulation routines in ffs(3). Maybe some can be of use here?

Learned this man pages, but it is more about bit-sequences finding. Here I should only fill first parts of bitmaps. So, can not find how it is possible to apply it here.

pfg added inline comments.Jun 18 2017, 7:08 PM

sys/fs/ext2fs/ext2_alloc.c
713 ↗	(On Diff #29709)	This looks strange: X / Y / 8 = 8 * X / Y. Also ... should cg be unsigned?
sys/fs/ext2fs/ext2_csum.c
96 ↗	(On Diff #29709)	offsetof(3) returns a size_t type: it could overflow int.
121 ↗	(On Diff #29709)	i should be unsigned as it is used to index an unsigned number.
141 ↗	(On Diff #29709)	Thia should be an unsigned int since e2fs_gcount is unsigned.
sys/fs/ext2fs/ext2_vfsops.c
397 ↗	(On Diff #29709)	Typo (Verify).

Note to self: I think this feature may work on ext3 so it can be tested with tune2fs.

Comments fixes.

sys/fs/ext2fs/ext2_alloc.c
713 ↗	(On Diff #29709)	Also ... should cg be unsigned? cg is signed everywhere, I can try to change it to unsigned, but seems like, it should be another patch.