From: "J. Bruce Fields" Subject: Re: [PATCH] ext4: turn on i_version updates by default Date: Tue, 15 May 2012 08:35:50 -0400 Message-ID: <20120515123550.GA7053@fieldses.org> References: <20120514140618.GA29902@fieldses.org> <9124E59E-2479-4C32-A528-3237B48DEC01@dilger.ca> <1337036918.2522.32.camel@lade.trondhjem.org> <20120514235432.GA3199@fieldses.org> <20120515103021.GD22359@quack.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andreas Dilger , "Myklebust, Trond" , Theodore Ts'o , "linux-ext4@vger.kernel.org" , "linux-nfs@vger.kernel.org" , "linux-fsdevel@vger.kernel.org" To: Jan Kara Return-path: Received: from fieldses.org ([174.143.236.118]:59996 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758666Ab2EOMgR (ORCPT ); Tue, 15 May 2012 08:36:17 -0400 Content-Disposition: inline In-Reply-To: <20120515103021.GD22359@quack.suse.cz> Sender: linux-ext4-owner@vger.kernel.org List-ID: 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(). So maybe replace that futimes() by a one-byte write? --b.