On Sun, Feb 02, 2014 at 02:45:58PM +0900, Namjae Jeon wrote: > From: Namjae Jeon <namjae.jeon@xxxxxxxxxxx> > > This testcase(001) tries to test various corner cases > for fcollapse range functionality over different type of extents. > > Signed-off-by: Namjae Jeon <namjae.jeon@xxxxxxxxxxx> > Signed-off-by: Ashish Sangwan <a.sangwan@xxxxxxxxxxx> Couple of things: > -c "$map_cmd -v" $testfile | $filter_cmd > [ $? -ne 0 ] && die_now > _md5_checksum $testfile > @@ -415,10 +425,10 @@ _test_generic_punch() > if [ "$remove_testfile" ]; then > rm -f $testfile > fi > - $XFS_IO_PROG -f -c "truncate 20k" \ > - -c "$alloc_cmd 0 8k" \ > - -c "pwrite 8k 8k" $sync_cmd \ > - -c "$zero_cmd 4k 8k" \ > + $XFS_IO_PROG -f -c "truncate $(($multiple * 20))k" \ > + -c "$alloc_cmd 0 $(($multiple * 8))k" \ > + -c "pwrite $(($multiple * 8))k $(($multiple * 8))k" $sync_cmd \ > + -c "$zero_cmd $(($multiple * 4))k $(($multiple * 8))k" \ > -c "$map_cmd -v" $testfile | $filter_cmd This is unreadable, and therefore I'd consider that these changes render _test_generic_punch unmaintainable. Either it needs tobe factored to be more readable, or we need a more readable way of representing the offsets and sizes if we want them to be variable. For example: _4k="$((multiple * 4))k" _8k="$((multiple * 8))k" _20k="$((multiple * 20))k" leads to: $XFS_IO_PROG -f -c "truncate $_20k" \ -c "$alloc_cmd 0 $_8k" \ -c "pwrite $_8k $_8k" $sync_cmd \ -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd which is still readable and allows us to arbitrarily scale the parameters. It even allows us to handle different filesystem block sizes if we really want to.... > -c "$map_cmd -v" $testfile | $filter_cmd > [ $? -ne 0 ] && die_now > _md5_checksum $testfile > > + # If zero_cmd is fcollpase, don't check unaligned offsets > + if [ "$zero_cmd" == "fcollapse" ]; then > + if [ "$remove_testfile" ]; then > + rm -f $testfile > + rm -f $testfile.2 > + fi > + return > + fi No need to remove the test files here - we remove them at test startup to ensure we have a known initial state.... > +0: [0..63]: extent > +bb7df04e1b0a2570657527a7e108ae23 > + 13. data -> unwritten -> data > +0: [0..63]: extent > +0f0151cbed83e4bf6e5bde26e82ab115 > + 14. data -> hole @ EOF > +fallocate: Invalid argument > +0: [0..159]: extent This error appears in all the golden outputs. If it's correct, then perhaps it should be filtered out or commented somewhere to explain why it is expected. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html