linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] writeback fixes - slow unmount and others
@ 2010-06-08 16:14 Christoph Hellwig
  2010-06-08 16:14 ` [PATCH 1/6] writeback: fix writeback completion notifications Christoph Hellwig
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Christoph Hellwig @ 2010-06-08 16:14 UTC (permalink / raw)
  To: axboe; +Cc: linux-fsdevel

I've started looking into the slow unmount issue again, and digged a
bit deeper into the writeback code to look at the root cause.

All inode writeback is performed by writeback_inodes_wb, which
has the following callgraph (annotated by a few important facts):

writeback_inodes_wb
  <- writeback_inodes_wbc
       <- bdi_flush_io			(WB_SYNC_NONE, no sb)
       <- balance_dirty_pages		(WB_SYNC_NONE, no sb)
  <- wb_writeback
       <- wb_check_old_data_flush	(WB_SYNC_NONE, no sb)
       <- wb_do_writeback
           <- bdi_writeback_task	\
	   <- bdi_start_fn		| take bdi_work item from list
	   <- bdi_forker_task		/

So we either get a bdi_work item submitted by bdi_queue_work, or
we do WB_SYNC_NONE I/O ourselves without specifying a superblock.

For bdi_queue_work the same annotated callchain looks like:

bdi_queue_work
  <- bdi_sync_writeback			(WB_SYNC_ALL, mandatory sb, onstack)
       <- sync_inodes_sb		(..)
  <- bdi_alloc_queue_work
       <- bdi_start_writeback		(WB_SYNC_NONE, optional sb)
            <- balance_dirty_pages	(..          , no sb)
	    <- laptop_mode_timer_fn	(..          , no sb)
	    <- writeback_inodes_sb	(..          , mandatory sb)
       <- bdi_writeback_all		(WB_SYNC_NONE, optional sb)
            <- wakeup_flusher_threads	(..          , no sb)

So in general we can say that the case of WB_SYNC_NONE without a
superblock is the normal case.  And now if we look at the callers of
the functions that take a superblock we see that this is basically
sync_filesystem, which always has s_umount held.  So instead of
trying to add hacks for when it is held, we can assume that we
always have it held if a superblock is specified, and fix up the
two callers inside filesystems that crept in recently to also
hold it.  With that we can replace various ad-hoc checks with
a very clear scheme that only checks the oncase caller, and if
we have a superblock or not after switching all writeback
that has a superblock to submit the I/O on stack.

After my series the annotated callgraph for bdi_queue_work is the
following:


bdi_queue_work
  <- bdi_queue_work_onstack		 (WB_SYNC_ALL/NONE, mandatory sb, onstack)
       <- sync_inodes_sb		 (WB_SYNC_ALL, ..)
       <- writeback_inodes_sb		 (WB_SYNC_NONE, ..)
  <- bdi_alloc_queue_work
       <- bdi_start_writeback		 (WB_SYNC_NONE, no sb)
       <- bdi_start_background_writeback (WB_SYNC_NONE, no sb)
       <- wakeup_flusher_threads	 (WB_SYNC_NONE, no sb)

So it's a lot simpler and just as important flatter.  But when we look
how we use the onstack / sb specified callers I'm not sure this is the
optimal case yet.  We're beeing called from synchronous callers (
we need to wait for completion for the first WB_SYNC_NONE pass later on,
too), so there really is no need to hand this work off to the flusher
threads.  In the end it might be much simpler to just issue this I/O
from the caller context and remove the code for the on-stack
submissions, but that's left for a later patch.

^ permalink raw reply	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-06-15 18:05 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-08 16:14 [PATCH 0/6] writeback fixes - slow unmount and others Christoph Hellwig
2010-06-08 16:14 ` [PATCH 1/6] writeback: fix writeback completion notifications Christoph Hellwig
2010-06-08 19:50   ` Jens Axboe
2010-06-15 17:25   ` Jan Kara
2010-06-15 17:30     ` Christoph Hellwig
2010-06-08 16:14 ` [PATCH 2/6] writeback: queue work on stack in writeback_inodes_sb Christoph Hellwig
2010-06-08 19:51   ` Jens Axboe
2010-06-08 16:14 ` [PATCH 3/6] writeback: enforce s_umount locking " Christoph Hellwig
2010-06-15 17:54   ` Jan Kara
2010-06-15 17:59     ` Christoph Hellwig
2010-06-15 18:04       ` Jan Kara
2010-06-08 16:14 ` [PATCH 4/6] writeback: fix writeback_inodes_wb from writeback_inodes_sb Christoph Hellwig
2010-06-08 19:51   ` Jens Axboe
2010-06-09 12:25     ` Christoph Hellwig
2010-06-09 12:29       ` Jens Axboe
2010-06-08 16:15 ` [PATCH 5/6] writeback: simplify wakeup_flusher_threads Christoph Hellwig
2010-06-08 19:51   ` Jens Axboe
2010-06-08 16:15 ` [PATCH 6/6] writeback: simplify and split bdi_start_writeback Christoph Hellwig
2010-06-08 19:52   ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).