On Tue 15-05-12 08:35:50, J. Bruce Fields wrote: > On Tue, May 15, 2012 at 12:30:21PM +0200, Jan Kara wrote: > > On Mon 14-05-12 19:54:32, J. Bruce Fields wrote: > > > On Mon, May 14, 2012 at 05:33:04PM -0600, Andreas Dilger wrote: > > > > I said as much in another reply - that once i_version is used on > > > > a filesystem, it should be made "sticky" (i.e. permanently enabled > > > > for that filesystem). However, until that time it shouldn't be > > > > enabled just because it might one day be used. > > > > > > > > Even better than just blindly bumping the i_version on every change, > > > > it would be better to have users of i_version (i.e. knfsd) flag the > > > > inode with "needs i_version update" then read the version. When the > > > > filesystem/VFS bumps i_version the next time it can clear this flag > > > > and not update i_version again until after the next time i_version > > > > is actually used. > > > > > > I really don't want to do anything more complicated than necessary. > > > > > > What would be the worst-case test for the extra inode dirtying, so we > > > can see what the numbers actually are? > > Something like: > > > > int fd, i; > > struct timeval tv[2]; > > > > fd = open("file", O_CREAT | O_RDWR, 0644); > > if (fd < 0) > > return 1; > > for (i = 0; i < 1000000; i++) { > > gettimeofday(tv); > > tv[1] = tv[0]; > > if (futimes(fd, tv) < 0) > > return 1; > > } > > return 0; > > > > And see how long does it take with and without i_version? > > The complaint I hear from Andreas is that we'll cause file_update_time() > to call mark_inode_dirty() more often. > > I don't believe futimes() calls file_update_time(). Yeah, right. I didn't check and I was wrong... > So maybe replace that futimes() by a one-byte write? Yes, "pwrite(fd, "data", 1, 0);" should be then a proper test instead of futimes(). Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html