Re: [patch 2/4] fs: fsync inode dirty race fix

[Nick Piggin <npiggin@gmail.com>]

On Tue, Nov 23, 2010 at 12:23 AM, Christoph Hellwig <hch@infradead.org> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html