Re: Metadata in sys_sync_file_range and fadvise(DONTNEED)

[Chad Talbott <ctalbott@google.com>]

On Sun, Nov 2, 2008 at 2:45 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Oct 31, 2008 at 01:54:14PM -0700, Chad Talbott wrote:
>> We are looking at adding calls to posix_fadvise(DONTNEED) to various
>> data logging routines.  This has two benefits:
>>
>>   - frequent write-out -> shorter queues give lower latency, also disk
>>     is more utilized as writeout begins immediately
>>
>>   - less useless stuff in page cache
>>
>> One problem with fadvise() (and ext2, at least) is that associated
>> metadata isn't scheduled with the data. So, for a large log file with
>> a high append rate, hundreds of indirect blocks are left to be written
>> out by periodic writeback.  This metadata consists of single blocks
>> spaced by 4MB, leading to spikes of very inefficient disk utilization,
>> deep queues and high latency.
>
> Sounds like a filesystem bug to me, not a problem with
> posix_fadvise(DONTNEED).

Agreed that the right fix is not to hack fadvise().  If the boundary
page mechanism can't be made to work, It looks like right thing might
be to modify ext2_writepages to opportunistically write dirty metadata
in holes between dirty data.  For post-ext2 filesystems which attempt
to provide transactional semantics, this is probably not acceptable.

>> Andrew suggests a new SYNC_FILE_RANGE_METADATA flag for
>> sys_sync_file_range(), and leaving posix_fadvise() alone.
>
> What is the interface that a filesystem will see? No filesystem has
> a "metadata sync" method - is this going to fall through to some new
> convoluted combination of writeback flags to an inode/mapping
> that more filesystems than not can get wrong?

Good point, coupled with metadata/data ordering and your argument
below, a decent argument against exposing this interface.

> FWIW, sys_sync_file_range() is fundamentally broken for data
> integrity writeback - at no time does it call a filesystem method
> that can result in a barrier I/O being issued to disk after
> writeback is complete. So, unlike fsync() or fdatasync(), the data
> can still be lost after completion due to power failure on drives
> with volatile write caches....

Seems to be true.  I'm not currently concerned with sync_file_range
for data integrity, so I'm going to punt on this issue.

If the consensus is against exposing a "sync metadata" interface, I'm
fine with ext2 silently updating metadata alongside neighboring data
in *either* posix_fadvise() or sync_file_range.   Either way, does it
seem reasonable for posix_fadvise(DONTNEED) to call
__filemap_fdatawrite_range to do its work?  This is the same path that
sync_file_range uses.  I prefer this to the current behavior of
ignoring the passed range and initiating writeback on the entire file.

Chad