* On Mon, Apr 18, 2011 at 10:19:12AM +0300, Artem Bityutskiy <Artem.Bityutskiy@xxxxxxxxx> wrote:
On Sun, 2011-04-17 at 21:53 +0530, Raghavendra D Prabhu wrote:In the function bdi_wakeup_thread_delayed, no checks are performed on dirty_writeback_interval unlike other places and timeout is being set to zero as result, thus defeating the purpose. So, I have changed it to be passed default value of interval which is 500 centiseconds, when it is set to zero. I have also verified this and tested it.
Signed-off-by: Raghavendra D Prabhu <rprabhu@xxxxxxxxxxx>If dirty_writeback_interval then the periodic write-back has to be disabled. Which means we should rather do something like this: diff --git a/mm/backing-dev.c b/mm/backing-dev.c index 0d9a036..f38722c 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -334,10 +334,12 @@ static void wakeup_timer_fn(unsigned long data) */ void bdi_wakeup_thread_delayed(struct backing_dev_info *bdi) { - unsigned long timeout; + if (dirty_writeback_interval) { + unsigned long timeout; - timeout = msecs_to_jiffies(dirty_writeback_interval * 10); - mod_timer(&bdi->wb.wakeup_timer, jiffies + timeout); + timeout = msecs_to_jiffies(dirty_writeback_interval * 10); + mod_timer(&bdi->wb.wakeup_timer, jiffies + timeout); + } } I do not see why you use 500 centisecs instead - I think this is wrong.--- mm/backing-dev.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/mm/backing-dev.c b/mm/backing-dev.c index befc875..d06533c 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -336,7 +336,10 @@ void bdi_wakeup_thread_delayed(struct backing_dev_info *bdi) { unsigned long timeout;
- timeout = msecs_to_jiffies(dirty_writeback_interval * 10); + if (dirty_writeback_interval) + timeout = msecs_to_jiffies(dirty_writeback_interval * 10); + else + timeout = msecs_to_jiffies(5000); mod_timer(&bdi->wb.wakeup_timer, jiffies + timeout); }
Hi, I have set it to 500 centisecs as that is the default value of dirty_writeback_interval. I used this logic for following reason: the purpose for which dirty_writeback_interval is set to 0 is to disable periodic writeback (http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/fs/fs-writeback.c#L818) , whereas here (in bdi_wakeup_thread_delayed) it is being used for a different purpose -- to delay the bdi wakeup in order to reduce context switches for dirty inode writeback. Regarding the change you made: in that case won't it end up disabling the timer altogether ? which shouldn't happen given the original purpose of defining dirty_writeback_interval to zero.
Attachment:
pgphRg9sQFaGD.pgp
Description: PGP signature