Re: [PATCH 1/1] Add check for dirty_writeback_interval in bdi_wakeup_thread_delayed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* 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


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]