On Mon, 4 Oct 2010, Lukas Czerner wrote: > Hi Ted, > > first of all, thank you very much for tracking down those issues and for > all the improvement you have done on this. Now, I have some questions > about changes you have introduced with this patch. > > On Sun, 3 Oct 2010, Ted Ts'o wrote: > > > I've made some more changes. This version updates the timing control. > > The major changes are: > > > > 1) Time the it takes to clear the inode table with a barrier (once), > > and then use it for the rest of the block groups for that file system. > > So if I understand this right it means that we measure the time it takes > to zeroout inode table just once (set the lr_timeout) and then we use > this value for all the following zeroouts. > > Initially I have done this "measuring time" thing to adaptively balance > the load it generates and thus do not disturb other ongoing I/O very > much. So this change does not really make sense to me, because when we > measure the time right after the mount (just once) and the system is > relatively still we end up with rather small lr_timeout and then, when > system is under heavy load it will keep the same zeroout rate as when > system was still - resulting in much more impact on performance than my > previous solution. > > Conversely, when the system is under heavy load when the filesystem > with init_itable option is mounted the zeroing will proceed very slowly > even if the system is relatively still later on. > > > > > 2) s_li_wait_nult wasn't getting defaulted, so we weren't waiting any > > time at all between sb_issue_zeroout calls. > > Actually it is getting defaulted: > > + case Opt_init_inode_table: > + set_opt(sbi->s_mount_opt, INIT_INODE_TABLE); > + if (args[0].from) { > + if (match_int(&args[0], &option)) > + return 0; > + } else > + option = EXT4_DEF_LI_WAIT_MULT; > + if (option < 0) > + return 0; > + sbi->s_li_wait_mult = option; > + break; Ok, this is not right:) and it is probably why do you thought that it was not getting defaulted at all (because it actually did not). It should be: case Opt_init_inode_table: set_opt(sbi->s_mount_opt, INIT_INODE_TABLE); option = EXT4_DEF_LI_WAIT_MULT; if (args[0].from) { if (match_int(&args[0], &option)) return 0; if (option < 0) return 0; sbi->s_li_wait_mult = option; break; > > EXT4_DEF_LI_WAIT_MULT is the default value for s_li_wait_mult. > -Lukas -- 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