lkml.org 
[lkml]   [2012]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V2] writeback: fix hung_task alarm when sync block
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@gmail.com> 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


\
 
 \ /
  Last update: 2012-06-14 04:01    [W:0.069 / U:3.052 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site