On Fri, Oct 07, 2011 at 10:40:55PM +0200, Jan Kara wrote: > wakeup_flusher_threads(0) will queue work doing complete writeback for each > flusher thread. Thus there is not much point in submitting another work doing > full inode WB_SYNC_NONE writeback by writeback_inodes_sb(). > > After this change it does not make sense to call nonblocking ->sync_fs and > block device flush before calling sync_inodes_sb() because > wakeup_flusher_threads() is completely asynchronous and thus these functions > would be called in parallel with inode writeback running which will effectively > void any work they do. So we move sync_inodes_sb() call before these two > functions. > wakeup_flusher_threads(0); > - iterate_supers(sync_inodes_one_sb, &nowait); > + iterate_supers(sync_inodes_one_sb, &wait); > iterate_supers(sync_fs_one_sb, &nowait); > sync_all_bdevs(nowait); > - iterate_supers(sync_inodes_one_sb, &wait); > iterate_supers(sync_fs_one_sb, &wait); > sync_all_bdevs(wait); > if (unlikely(laptop_mode)) This looks a bit half-assed to me. Why do we still do the non-blocking sync_all_bdevs call? This really only starts async writeback on the block device inodes, which wakeup_flusher_threads already did. Similarly I don't think the non-blocking ->sync_fs call really make much sense anymore here. Also we really need some good comment on why the order is like the one chose here in this function. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
