On Wed, Jun 13, 2012 at 10:48:40PM +0800, Fengguang Wu wrote: >Hi Jeff, > >On Wed, Jun 13, 2012 at 10:27:50AM -0400, Jeff Moyer wrote: >> Wanpeng Li <liwp.linux@xxxxxxxxx> writes: >> >> > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c >> > index f2d0109..df879ee 100644 >> > --- a/fs/fs-writeback.c >> > +++ b/fs/fs-writeback.c >> > @@ -1311,7 +1311,11 @@ void writeback_inodes_sb_nr(struct super_block *sb, >> > >> > WARN_ON(!rwsem_is_locked(&sb->s_umount)); >> > bdi_queue_work(sb->s_bdi, &work); >> > - wait_for_completion(&done); >> > + if (sysctl_hung_task_timeout_secs) >> > + while (!wait_for_completion_timeout(&done, HZ/2)) >> > + ; >> > + else >> > + wait_for_completion(&done); >> > } >> > EXPORT_SYMBOL(writeback_inodes_sb_nr); >> >> Is it really expected that writeback_inodes_sb_nr will routinely queue >> up more than 2 seconds worth of I/O (Yes, I understand that it isn't the >> only entity issuing I/O)? > >Yes, in the case of syncing the whole superblock. >Basically sync() does its job in two steps: > >for all sb: > writeback_inodes_sb_nr() # WB_SYNC_NONE > sync_inodes_sb() # WB_SYNC_ALL > >> For devices that are really slow, it may make >> more sense to tune the system so that you don't have too much writeback >> I/O submitted at once. Dropping nr_requests for the given queue should >> fix this situation, I would think. > >The worried case is about sync() waiting > > (nr_dirty + nr_writeback) / write_bandwidth > >time, where it is nr_dirty that could grow rather large. > >For example, if dirty threshold is 1GB and write_bandwidth is 10MB/s, >the sync() will have to wait for 100 seconds. If there are heavy >dirtiers running during the sync, it will typically take several >hundreds of seconds (which looks not that good, but still much better >than being livelocked in some old kernels).. > >> This really feels like we're papering over the problem. > >That's true. The majority users probably don't want to cache 100s >worth of data in memory. It may be worthwhile to add a new per-bdi >limit whose unit is number-of-seconds (of dirty data). Hi Fengguang, Maybe we have already have a threshold "dirty_expire_interval" to ensure pages will not dirty more than 30 seconds. Why should add a similar variable ? I think per-bdi flusher will try its best to flush dirty pages when waken up, just because the backing storages is too slow. :-) Best Regards, Wanpeng Li > >Thanks, >Fengguang -- 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
