On Thu, Jun 25, 2015 at 02:46:44PM -0400, J. Bruce Fields wrote: > > Does this sound reasonable? > > Just to make sure I understand, the logic is something like: > > to read the i_version: > > inode->i_version_seen = true; > return inode->i_version > > to update the i_version: > > /* > * If nobody's seen this value of i_version then we can > * keep using it, otherwise we need a new one: > */ > if (inode->i_version_seen) > inode->i_version++; > inode->i_version_seen = false; Yep, that's what I was proposing. > Looks OK to me. As I say I'd expect i_version_seen == true to end up > being the common case in a lot of v4 workloads, so I'm more skeptical of > the claim of a performance improvement in the v4 case. Well, so long as we require i_version to be committed to disk on every single disk write, we're going to be trading off: * client-side performance of the advanced NFSv4 cacheing for reads * server-side performance for writes * data robustness in case of the server crashing and the client-side cache getting out of sync with the server after the crash I don't see any way around that. (So for example, with lazy mtime updates we wouldn't be updating the inode after every single non-allocating write; enabling i_version updates will trash that optimization.) I just want to reduce to a bare minimum the performance hit in the case where NFSv4 exports are not being used (since that is true in a very *large* number of ext4 deployments --- i.e., every single Android handset using ext4 :-), such that we can leave i_version updates turned on by default. > Could maintaining the new flag be a significant drag in itself? If not, > then I guess we're not making things any worse there, so fine. I don't think so; it's a bit in the in-memory inode, so I don't think that should be an issue. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
