On Mon 24-08-15 16:24:25, Dave Chinner wrote:
> On Mon, Aug 24, 2015 at 11:18:16AM +0800, Eryu Guan wrote:
> > On Mon, Aug 24, 2015 at 11:11:23AM +1000, Dave Chinner wrote:
> > >
> > > Eryu, can you change the way you run the event trace to be:
> > >
> > > $ sudo trace-cmd <options> -o <outfile location> ./check <test options>
> > >
> > > rather than running the trace as a background operation elsewhere?
> > > Maybe that will give better results.
> >
> > The results are here
> >
> > http://128.199.137.77/writeback-v3/
<snip>
> What I can't see in the traces is where sync is doing a blocking
> sync pass on the fileystem. The wbc control structure being passed
> to XFS is:
>
> wbc_writepage: bdi 253:0: towrt=45569 skip=0 mode=0 kupd=0 bgrd=0
> reclm=0 cyclic=0 start=0x0 end=0x7fffffffffffffff
>
> Which is not coming from sync_inodes_sb() as the sync mode is
> incorrect (i.e. not WB_SYNC_ALL). It looks to me that writeback is
> coming from a generic bdi flusher command rather than a directed
> superblock sync. i.e. through wakeup_flusher_threads() which sets:
>
> work->sync_mode = WB_SYNC_NONE;
> work->nr_pages = nr_pages;
> work->range_cyclic = range_cyclic;
> work->reason = reason;
> work->auto_free = 1;
>
> as the reason is "sync":
>
> sync-18849 writeback_queue: bdi 253:0: sb_dev 0:0
> nr_pages=308986 sync_mode=0 kupdate=0 range_cyclic=0 background=0 reason=sync
> sync-18849 writeback_queue: bdi 253:0: sb_dev 253:1
> nr_pages=9223372036854775807 sync_mode=1 kupdate=0 range_cyclic=0
> background=0 reason=sync
> ....
> kworker/u8:1-1563 writeback_exec: bdi 253:0: sb_dev 0:0
> nr_pages=308986 sync_mode=0 kupdate=0 range_cyclic=0 background=0 reason=sync
> kworker/u8:1-1563 writeback_start: bdi 253:0: sb_dev 0:0
> nr_pages=308986 sync_mode=0 kupdate=0 range_cyclic=0 background=0 reason=sync
>
> The next writeback_queue/writeback_exec tracepoint pair are:
>
> ....
> kworker/2:1-17163 xfs_setfilesize: dev 253:6 ino 0xef6506 isize
> 0xa00000 disize 0x0 offset 0x0 count 10481664
> kworker/2:1-17163 xfs_setfilesize: dev 253:6 ino 0xef6506 isize
> 0xa00000 disize 0x9ff000 offset 0x9ff000 count 4096
> sync-18849 wbc_writepage: bdi 253:0:
> towrt=9223372036854775798 skip=0 mode=1 kupd=0 bgrd=0 reclm=0 cyclic=0
> start=0x0 end=0x7fffffffffffffff
> sync-18849 wbc_writepage: bdi 253:0:
> towrt=9223372036854775797 skip=0 mode=1 kupd=0 bgrd=0 reclm=0 cyclic=0
> start=0x0 end=0x7fffffffffffffff
> sync-18849 wbc_writepage: bdi 253:0:
> towrt=9223372036854775796 skip=0 mode=1 kupd=0 bgrd=0 reclm=0 cyclic=0
> start=0x0 end=0x7fffffffffffffff
> sync-18849 wbc_writepage: bdi 253:0:
> towrt=9223372036854775795 skip=0 mode=1 kupd=0 bgrd=0 reclm=0 cyclic=0
> start=0x0 end=0x7fffffffffffffff
> umount-18852 writeback_queue: bdi 253:0: sb_dev 253:6
> nr_pages=22059 sync_mode=0 kupdate=0 range_cyclic=0 background=0 reason=sync
> kworker/u8:1-1563 writeback_exec: bdi 253:0: sb_dev 253:6
> nr_pages=22059 sync_mode=0 kupdate=0 range_cyclic=0 background=0 reason=sync
> ....
>
> which shows unmount being the next writeback event queued and
> executed after the IO completions have come in (that missed the
> log). What is missing is the specific queue/exec events for
> sync_sb_inodes() from the sync code for each filesystem.
Bah, I see the problem and indeed it was introduced by commit e79729123f639
"writeback: don't issue wb_writeback_work if clean". The problem is that
we bail out of sync_inodes_sb() if there is no dirty IO. Which is wrong
because we have to wait for any outstanding IO (i.e. call wait_sb_inodes())
regardless of dirty state! And that also explains why Tejun's patch fixes
the problem because it backs out the change to the exit condition in
sync_inodes_sb().
So Tejun's patch from this thread is indeed fixing the real problem but the
comment in sync_inodes_sb() should be fixed to mention wait_sb_inodes()
must be called in all cases... Tejun, will you fixup the comment please?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
|