Re: [PATCH] ext4: Fix up range_end offset in range cyclic writeback

Shantanu Goel
2012-01-09T16:20:54Z

> So this was a performance issue that led you to this investigation?
> How much of a performance issue are we talking about?

It's a performance issue in the sense that background flush of the file can take much longer (we observed > 5 mins in some instances) than the 30 second kupdate interval.

> If it's a correctness issue that might lead to data loss because
> fsync() wasn't doing its jobs, that's obviously higher priority than a
> performance issue.

fsync() is not affected and works correctly with or without the patch since it does not use the range_cyclic writeback mode, AFAICT.  The only "correctness" issue would be if an application relies on kupdate which uses range_cyclic writeback to periodically flush the file as governed by the /proc/sys/vm/dirty_writeback_centisecs tunable.  We have some applications which lost more data than they expected due to the writeback not happening at expected intervals.  One could argue the bug is in the application since it could have guaranteed the file was flushed by using fsync hence my quotes around correctness.  Unfortunately, modifying the application is not an easy option for us to pursue at this time.  We just wish to have the patch merged upstream and the exact timing is not that critical.

> Again, I just wanted to characterize this correctly in the commit ---
> BTW, this is good stuff to include in the commit description so in the
> future, developers trying to go through the history can understand why
> a particular patch is important (perhaps to backport into an
> enterprise distro release, etc.)

Sure, will do in future.

Thanks,
Shantanu