On 2012-05-15, at 3:00 PM, J. Bruce Fields wrote: > On Tue, May 15, 2012 at 04:05:34PM -0400, Josef Bacik wrote: >> On Tue, May 15, 2012 at 01:55:33PM -0600, Andreas Dilger wrote: >>> It should be fairly straight forward to have a flag set in the ext4 >>> superblock (s_state flag?) that indicates that the filesystem has >>> been exported via NFS. There might be other optimizations that can >>> be done based on this (e.g. avoid some of the directory cookie >>> hijinx that are only needed if NFS has exported the filesystem and >>> needs to keep persistent cookies across reboots). >>> >>> I think that the ext4_mark_inode_dirty() performance problem could >>> be at least partially fixed by deferring the copy of in-core inode >>> to on-disk inode to use a journal commit callback. This is far more >>> work than just setting a flag in the superblock, but it has the >>> potential to _improve_ performance rather than make it worse. > > Could you give any more pointers for an ext4 ignoramus? (Where *is* the > journal commit code that would need the callback? And where is the copy > currently done?) The ext4_mark_inode_dirty() starts a transaction handle, and calls ext4_mark_iloc_dirty->ext4_do_update_inode() to do the copying and dirtying of the block. The JBD2 commit callback was added for OCFS2 to do the checksums of the blocks only once, instead of for each modification. IIRC, this is done by a callback on struct journal_head (one per buffer_head), but I haven't looked into all of the details. >> Yeah Btrfs doesn't have this sort of problem since we delay inode >> updating sinc it is so costly, we simply let it hang around in the >> in-core inode until we feel like updating it at some point down the >> road. I'll put together a feature flag or something to make it be >> enabled for always if somebody turns it on. > > Thanks for looking at this. > > A feature flag would be an improvement over a mount option. > > If the flag makes a noticeable difference to performance, then it makes > me nervous toggling it automatically. And what will we do if statx > starts returning i_version to userspace? As mentioned in another post, all statxat() fields are optional. Cheers, Andreas -- 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
