Messages in this thread | | | Date | Fri, 14 Mar 2014 23:07:04 +0900 | Subject | Re: [PATCH 1/1] fs/mpage.c: forgotten WRITE_SYNC in case of data integrity write | From | Roman Peniaev <> |
| |
Hello, Tejun.
On Fri, Mar 14, 2014 at 10:07 PM, Tejun Heo <tj@kernel.org> wrote: > Hello, Andrew. > > On Thu, Mar 13, 2014 at 02:34:56PM -0700, Andrew Morton wrote: >> Jens isn't talking to us. Tejun, are you able explain REQ_SYNC? > > It has nothing to do with data integrity. It's just a hint telling > the block layer that someone is waiting for the IO so it'd be a good > idea to prioritize it. For example, nothing visible to userland > really waits for periodic writebacks, so we can delay their processing > to prioritize, for example, READs triggered from a page fault, which > is obviously causing userland visible latency. > > Block layer treats all READs as REQ_SYNC and also allows upper layers > to mark some writes REQ_SYNC for cases where somebody is waiting for > the write to complete for cases like flush(2). > >> From: Roman Pen <r.peniaev@gmail.com> >> Subject: fs/mpage.c: forgotten WRITE_SYNC in case of data integrity write >> >> In case of wbc->sync_mode == WB_SYNC_ALL we need to do data integrity >> write, thus mark request as WRITE_SYNC. > > So, at least this patch description is very misleading. WRITE_SYNC > has *NOTHING* to do with data integrity. The only thing matters is > whether somebody is waiting for its completion or not.
The thing is that I started investigating WB_SYNC_ALL and WRITE_SYNC from the __filemap_fdatawrite_range call which has explicit comment:
* If sync_mode is WB_SYNC_ALL then this is a "data integrity" operation, as * opposed to a regular memory cleansing writeback.
So, in the commit msg I want to say that when we are doing integrity write (sync, fsync calls) we have to mark requests as WRITE_SYNC, and there is only one path (mpage writeback path) where WRITE_SYNC is missed. That was the idea. (not WRITE_SYNC is responsible for integrity write, but in case of integrity write)
Seems the following message should be better: When data inegrity operation (sync, fsync, fdatasync calls) happens writeback control is set to WB_SYNC_ALL. In that case all write requests are marked with WRITE_SYNC, but on mpage writeback path WRITE_SYNC is missed. This patch fixes this.
Is it ok, what do you think?
Also, could you please help me do understand how can I guarantee integrity in case of block device with big volatile cache and filesystem, which does not support REQ_FLUSH/FUA?
Even if I am doing fsync, block device will never receive any indication, that these requests should really be stored on device, or cache should be flushed right now (like REQ_FLUSH/FUA behaviour).
Seems it is impossible to do explicit block device cache flush if filesystem does not care about it.
Thanks.
-- Roman
> > Thanks. > > -- > tejun
| |