Re: [PATCH 2/2] UBIFS: start using hrtimers

Re: [PATCH 2/2] UBIFS: start using hrtimers

[Peter Zijlstra]

On Fri, 2009-05-29 at 18:09 +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Subject: [PATCH 2/2] UBIFS: start using hrtimers
> 
> UBIFS uses timers for write-buffer write-back. It is not
> crucial for us to write-back exactly on time. We are fine
> to write-back a little earlier or later. And this means
> we may optimize UBIFS timer so that it could be groped
> with a close timer event, so that the CPU would not be
> waken up just to do the write back. This is optimization
> to lessen power consumption, which is important in
> embedded devices UBIFS is used for.
> 
> hrtimers have a nice feature: they are effectively range
> timers, and we may defind the soft and hard limits for
> it. Standard timers do not have these feature. They may
> only be made deferrable, but this means there is effectively
> no hard limit. So, we will better use hrtimers.
> 
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> ---

Seems sane enough from a hrtimer POV, but isn't this already
functionality that the VFS/pdflush provide?

Re: [PATCH 2/2] UBIFS: start using hrtimers

[Artem Bityutskiy]

Peter Zijlstra wrote:
> On Fri, 2009-05-29 at 18:09 +0300, Artem Bityutskiy wrote:
>> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>> Subject: [PATCH 2/2] UBIFS: start using hrtimers
>>
>> UBIFS uses timers for write-buffer write-back. It is not
>> crucial for us to write-back exactly on time. We are fine
>> to write-back a little earlier or later. And this means
>> we may optimize UBIFS timer so that it could be groped
>> with a close timer event, so that the CPU would not be
>> waken up just to do the write back. This is optimization
>> to lessen power consumption, which is important in
>> embedded devices UBIFS is used for.
>>
>> hrtimers have a nice feature: they are effectively range
>> timers, and we may defind the soft and hard limits for
>> it. Standard timers do not have these feature. They may
>> only be made deferrable, but this means there is effectively
>> no hard limit. So, we will better use hrtimers.
>>
>> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>> ---
> 
> Seems sane enough from a hrtimer POV, but isn't this already
> functionality that the VFS/pdflush provide?

Yeah, VFS/pdflush takes care of the page-cache and inode cache,
and dirty superblocks. But additionally to this UBIFS has its
own small buffer of (usually) 2KiB size, we call it write-buffer.
This is a very important optimization for NAND flash. And we
have a separate timer to synchronize this small write-buffer.

I was also thinking to hook to VFS, which would mean creating
a fake inode representing our write-bufffer. But this would be
very hacky.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] UBIFS: start using hrtimers

[Artem Bityutskiy]

Peter Zijlstra wrote:
> On Fri, 2009-05-29 at 18:09 +0300, Artem Bityutskiy wrote:
>> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>> Subject: [PATCH 2/2] UBIFS: start using hrtimers
>>
>> UBIFS uses timers for write-buffer write-back. It is not
>> crucial for us to write-back exactly on time. We are fine
>> to write-back a little earlier or later. And this means
>> we may optimize UBIFS timer so that it could be groped
>> with a close timer event, so that the CPU would not be
>> waken up just to do the write back. This is optimization
>> to lessen power consumption, which is important in
>> embedded devices UBIFS is used for.
>>
>> hrtimers have a nice feature: they are effectively range
>> timers, and we may defind the soft and hard limits for
>> it. Standard timers do not have these feature. They may
>> only be made deferrable, but this means there is effectively
>> no hard limit. So, we will better use hrtimers.
>>
>> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>> ---
> 
> Seems sane enough from a hrtimer POV, but isn't this already
> functionality that the VFS/pdflush provide?

And I also have a longer term plan to switch the VFS/pdflush
timer to be a range hrtimer, BTW, just like you suggested
me once.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] Use hrtimers in UBIFS

[Artem Bityutskiy]

Hi,

Resending this to lkml. No comments from fsdevel:
http://marc.info/?l=linux-fsdevel&m=124351121304115&w=2

I guess no one cares, which is fine with me as long as my patch
which exports ktime_add_safe is accepted :-)

Could the relevant people accept the following patch please:
[PATCH 1/2] hrtimer: export ktime_add_safe
for 2.6.31 merge window?

I've CC'ed people suggested by:
$ scripts/get_maintainer.pl -f kernel/hrtimer.c

Artem.

[PATCH 1/2] hrtimer: export ktime_add_safe

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Subject: [PATCH 1/2] hrtimer: export ktime_add_safe

We want to use hrtimers in UBIFS (for write-buffer write-back timer).
We need the 'hrtimer_set_expires_range_ns()', which is an in-line
function which uses 'ktime_add_safe()'.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 kernel/hrtimer.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index cb8a15c..18f6906 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -332,6 +332,8 @@ ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs)
 	return res;
 }
 
+EXPORT_SYMBOL_GPL(ktime_add_safe);
+
 #ifdef CONFIG_DEBUG_OBJECTS_TIMERS
 
 static struct debug_obj_descr hrtimer_debug_descr;
-- 
1.6.0.6

[PATCH 2/2] UBIFS: start using hrtimers

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Subject: [PATCH 2/2] UBIFS: start using hrtimers

UBIFS uses timers for write-buffer write-back. It is not
crucial for us to write-back exactly on time. We are fine
to write-back a little earlier or later. And this means
we may optimize UBIFS timer so that it could be groped
with a close timer event, so that the CPU would not be
waken up just to do the write back. This is optimization
to lessen power consumption, which is important in
embedded devices UBIFS is used for.

hrtimers have a nice feature: they are effectively range
timers, and we may defind the soft and hard limits for
it. Standard timers do not have these feature. They may
only be made deferrable, but this means there is effectively
no hard limit. So, we will better use hrtimers.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/io.c    |   34 +++++++++++++++++++++-------------
 fs/ubifs/super.c |    6 +++---
 fs/ubifs/ubifs.h |   13 ++++++++-----
 3 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index e8e632a..bc58571 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -293,13 +293,14 @@ void ubifs_prep_grp_node(struct ubifs_info *c, void *node, int len, int last)
  *
  * This function is called when the write-buffer timer expires.
  */
-static void wbuf_timer_callback_nolock(unsigned long data)
+static enum hrtimer_restart wbuf_timer_callback_nolock(struct hrtimer *timer)
 {
-	struct ubifs_wbuf *wbuf = (struct ubifs_wbuf *)data;
+	struct ubifs_wbuf *wbuf = container_of(timer, struct ubifs_wbuf, timer);
 
 	wbuf->need_sync = 1;
 	wbuf->c->need_wbuf_sync = 1;
 	ubifs_wake_up_bgt(wbuf->c);
+	return HRTIMER_NORESTART;
 }
 
 /**
@@ -308,13 +309,12 @@ static void wbuf_timer_callback_nolock(unsigned long data)
  */
 static void new_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
 {
-	ubifs_assert(!timer_pending(&wbuf->timer));
+	ubifs_assert(!hrtimer_active(&wbuf->timer));
 
-	if (!wbuf->timeout)
+	if (!ktime_to_ns(wbuf->softlimit))
 		return;
-
-	wbuf->timer.expires = jiffies + wbuf->timeout;
-	add_timer(&wbuf->timer);
+	hrtimer_start_range_ns(&wbuf->timer, wbuf->softlimit, wbuf->delta,
+			       HRTIMER_MODE_REL);
 }
 
 /**
@@ -329,7 +329,7 @@ static void cancel_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
 	 * should be canceled.
 	 */
 	wbuf->need_sync = 0;
-	del_timer(&wbuf->timer);
+	hrtimer_cancel(&wbuf->timer);
 }
 
 /**
@@ -825,6 +825,7 @@ out:
 int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
 {
 	size_t size;
+	ktime_t hardlimit;
 
 	wbuf->buf = kmalloc(c->min_io_size, GFP_KERNEL);
 	if (!wbuf->buf)
@@ -845,14 +846,21 @@ int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
 	wbuf->sync_callback = NULL;
 	mutex_init(&wbuf->io_mutex);
 	spin_lock_init(&wbuf->lock);
-
 	wbuf->c = c;
-	init_timer(&wbuf->timer);
-	wbuf->timer.function = wbuf_timer_callback_nolock;
-	wbuf->timer.data = (unsigned long)wbuf;
-	wbuf->timeout = DEFAULT_WBUF_TIMEOUT;
 	wbuf->next_ino = 0;
 
+	hrtimer_init(&wbuf->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	wbuf->timer.function = wbuf_timer_callback_nolock;
+	/*
+	 * Make write-buffer soft limit to be 20% of the hard limit. The
+	 * write-buffer timer is allowed to expire any time between the soft
+	 * and hard limits.
+	 */
+	hardlimit = ktime_set(DEFAULT_WBUF_TIMEOUT_SECS, 0);
+	wbuf->delta = (DEFAULT_WBUF_TIMEOUT_SECS * NSEC_PER_SEC) * 2 / 10;
+	wbuf->softlimit = ktime_sub_ns(hardlimit, wbuf->delta);
+	hrtimer_set_expires_range_ns(&wbuf->timer,  wbuf->softlimit,
+				     wbuf->delta);
 	return 0;
 }
 
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 2ba4e83..3513cad 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -799,7 +799,7 @@ static int alloc_wbufs(struct ubifs_info *c)
 	 * does not need to be synchronized by timer.
 	 */
 	c->jheads[GCHD].wbuf.dtype = UBI_LONGTERM;
-	c->jheads[GCHD].wbuf.timeout = 0;
+	c->jheads[GCHD].wbuf.softlimit = ktime_set(0, 0);
 
 	return 0;
 }
@@ -1695,7 +1695,7 @@ static void ubifs_remount_ro(struct ubifs_info *c)
 
 	for (i = 0; i < c->jhead_cnt; i++) {
 		ubifs_wbuf_sync(&c->jheads[i].wbuf);
-		del_timer_sync(&c->jheads[i].wbuf.timer);
+		hrtimer_cancel(&c->jheads[i].wbuf.timer);
 	}
 
 	c->mst_node->flags &= ~cpu_to_le32(UBIFS_MST_DIRTY);
@@ -1755,7 +1755,7 @@ static void ubifs_put_super(struct super_block *sb)
 		if (c->jheads)
 			for (i = 0; i < c->jhead_cnt; i++) {
 				ubifs_wbuf_sync(&c->jheads[i].wbuf);
-				del_timer_sync(&c->jheads[i].wbuf.timer);
+				hrtimer_cancel(&c->jheads[i].wbuf.timer);
 			}
 
 		/*
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 0a8341e..1bf01d8 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -95,8 +95,8 @@
  */
 #define BGT_NAME_PATTERN "ubifs_bgt%d_%d"
 
-/* Default write-buffer synchronization timeout (5 secs) */
-#define DEFAULT_WBUF_TIMEOUT (5 * HZ)
+/* Default write-buffer synchronization timeout in seconds */
+#define DEFAULT_WBUF_TIMEOUT_SECS 5
 
 /* Maximum possible inode number (only 32-bit inodes are supported now) */
 #define MAX_INUM 0xFFFFFFFF
@@ -650,8 +650,10 @@ typedef int (*ubifs_lpt_scan_callback)(struct ubifs_info *c,
  * @io_mutex: serializes write-buffer I/O
  * @lock: serializes @buf, @lnum, @offs, @avail, @used, @next_ino and @inodes
  *        fields
+ * @softlimit: soft write-buffer timeout interval
+ * @delta: hard and soft timeouts delta (the timer expire inteval is @softlimit
+ *         and @softlimit + @delta)
  * @timer: write-buffer timer
- * @timeout: timer expire interval in jiffies
  * @need_sync: it is set if its timer expired and needs sync
  * @next_ino: points to the next position of the following inode number
  * @inodes: stores the inode numbers of the nodes which are in wbuf
@@ -678,8 +680,9 @@ struct ubifs_wbuf {
 	int (*sync_callback)(struct ubifs_info *c, int lnum, int free, int pad);
 	struct mutex io_mutex;
 	spinlock_t lock;
-	struct timer_list timer;
-	int timeout;
+	ktime_t softlimit;
+	unsigned long long delta;
+	struct hrtimer timer;
 	int need_sync;
 	int next_ino;
 	ino_t *inodes;
-- 
1.6.0.6

Re: [PATCH 1/2] hrtimer: export ktime_add_safe

[Ingo Molnar]

* Artem Bityutskiy <Artem.Bityutskiy@nokia.com> wrote:

> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Subject: [PATCH 1/2] hrtimer: export ktime_add_safe
> 
> We want to use hrtimers in UBIFS (for write-buffer write-back timer).
> We need the 'hrtimer_set_expires_range_ns()', which is an in-line
> function which uses 'ktime_add_safe()'.
> 
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> ---
>  kernel/hrtimer.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> index cb8a15c..18f6906 100644
> --- a/kernel/hrtimer.c
> +++ b/kernel/hrtimer.c
> @@ -332,6 +332,8 @@ ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs)
>  	return res;
>  }
>  
> +EXPORT_SYMBOL_GPL(ktime_add_safe);
> +

Feel free to queue this up in your tree - it's trivial enough.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH 1/2] hrtimer: export ktime_add_safe

[Artem Bityutskiy]

ext Ingo Molnar wrote:
> * Artem Bityutskiy <Artem.Bityutskiy@nokia.com> wrote:
> 
>> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>> Subject: [PATCH 1/2] hrtimer: export ktime_add_safe
>>
>> We want to use hrtimers in UBIFS (for write-buffer write-back timer).
>> We need the 'hrtimer_set_expires_range_ns()', which is an in-line
>> function which uses 'ktime_add_safe()'.
>>
>> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>> ---
>>  kernel/hrtimer.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
>> index cb8a15c..18f6906 100644
>> --- a/kernel/hrtimer.c
>> +++ b/kernel/hrtimer.c
>> @@ -332,6 +332,8 @@ ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs)
>>  	return res;
>>  }
>>  
>> +EXPORT_SYMBOL_GPL(ktime_add_safe);
>> +
> 
> Feel free to queue this up in your tree - it's trivial enough.
> 
> Acked-by: Ingo Molnar <mingo@elte.hu>

OK, thanks.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html