On Tue, Nov 23, 2010 at 12:23 AM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: >> ret = sync_mapping_buffers(inode->i_mapping); >> - if (!(inode->i_state & I_DIRTY)) >> - return ret; >> - if (datasync && !(inode->i_state & I_DIRTY_DATASYNC)) >> - return ret; >> - >> >> err = sync_inode_metadata(inode, 1); > > This makes fdatasync equivalent to fsync, which means a huge drop > in performance for database an virtualization performance. > > I think the right aproach is to extend the sync_inode_metadata to > writeback_single_inode chain with a datasync parameter so that we > can do the correct decision there. I was leaning towards using the new inode writeback helpers, then filesystems should use them to check inode dirty state without races. You could still move datasync into the writeback function, but it wouldn't be a requirement. More complex filesystem schemes seem to need to actually test these flags. > Note that there are a lot more fsync implementations than just > generic_file_fsync and exofs_fsync that have the same issue. Yes, they require patch 3/4, and a lot more testing than I had time to do tonight. I'll send out some patches tomorrow. -- 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