[RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Byungchul Park]

Hello Paul and folks,

I've thought the code should've been like the below since the range
checking of jiffies_till_first_fqs and jiffies_till_next_fqs everytime
in the loop of rcu_gp_kthread are unnecessary at all. However, it's ok
even if you don't think it's worth doing it.

Secondly, I also think jiffies_till_first_fqs = 0 is meaningless so
added checking and adjusting it as what's done on jiffies_till_next_fqs.
Thought?

Thank you in advance.
Byungchul

----->8-----
>From 67fecc15b44b2521de96de109782c04ce65afb85 Mon Sep 17 00:00:00 2001
From: Byungchul Park <byungchul.park@lge.com>
Date: Tue, 29 May 2018 15:49:26 +0900
Subject: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

Currently, the range of jiffies_till_first_fqs and jiffies_till_next_fqs
are always checked and adjusted in the loop of rcu_gp_kthread on runtime.
However, it would be better and enough to check them only on setting
them, so remove unnecessary checking and adjusting in the loop.

Additionally, add adjusting jiffies_till_first_fqs so guaranteed to be
greater than 0, which hasn't been done before.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 kernel/rcu/tree.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 4e96761..4964237 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -518,8 +518,31 @@ void rcu_all_qs(void)
 static ulong jiffies_till_next_fqs = ULONG_MAX;
 static bool rcu_kick_kthreads;
 
-module_param(jiffies_till_first_fqs, ulong, 0644);
-module_param(jiffies_till_next_fqs, ulong, 0644);
+static int param_set_fqs_jiffies(const char *val, const struct kernel_param *kp)
+{
+	ulong tmp;
+	int ret = kstrtoul(val, 0, &tmp);
+
+	if (ret < 0)
+		return ret;
+
+	if (tmp > HZ)
+		tmp = HZ;
+	else if (tmp < 1)
+		tmp = 1;
+
+	/* Prevent tearing */
+	WRITE_ONCE(*(ulong *)kp->arg, tmp);
+	return 0;
+}
+
+static struct kernel_param_ops fqs_jiffies_ops = {
+	.set = param_set_fqs_jiffies,
+	.get = param_get_ulong,
+};
+
+module_param_cb(jiffies_till_first_fqs, &fqs_jiffies_ops, &jiffies_till_first_fqs, 0644);
+module_param_cb(jiffies_till_next_fqs, &fqs_jiffies_ops, &jiffies_till_next_fqs, 0644);
 module_param(rcu_kick_kthreads, bool, 0644);
 
 /*
@@ -2129,10 +2152,6 @@ static int __noreturn rcu_gp_kthread(void *arg)
 		/* Handle quiescent-state forcing. */
 		first_gp_fqs = true;
 		j = jiffies_till_first_fqs;
-		if (j > HZ) {
-			j = HZ;
-			jiffies_till_first_fqs = HZ;
-		}
 		ret = 0;
 		for (;;) {
 			if (!ret) {
@@ -2167,13 +2186,6 @@ static int __noreturn rcu_gp_kthread(void *arg)
 				WRITE_ONCE(rsp->gp_activity, jiffies);
 				ret = 0; /* Force full wait till next FQS. */
 				j = jiffies_till_next_fqs;
-				if (j > HZ) {
-					j = HZ;
-					jiffies_till_next_fqs = HZ;
-				} else if (j < 1) {
-					j = 1;
-					jiffies_till_next_fqs = 1;
-				}
 			} else {
 				/* Deal with stray signal. */
 				cond_resched_tasks_rcu_qs();
-- 
1.9.1

Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Paul E. McKenney]

On Tue, May 29, 2018 at 04:23:36PM +0900, Byungchul Park wrote:
> Hello Paul and folks,
> 
> I've thought the code should've been like the below since the range
> checking of jiffies_till_first_fqs and jiffies_till_next_fqs everytime
> in the loop of rcu_gp_kthread are unnecessary at all. However, it's ok
> even if you don't think it's worth doing it.

Nice!

> Secondly, I also think jiffies_till_first_fqs = 0 is meaningless so
> added checking and adjusting it as what's done on jiffies_till_next_fqs.
> Thought?

Actually, jiffies_till_first_fqs == 0 is very useful for cases where
at least one CPU is expected to be idle and grace-period latency is
important.  In this case, doing the first scan immediately gets the
dyntick-idle state recorded immediately, getting the idle CPUs out of
the way of the grace period immediately.

So why not do this scan as part of grace-period initialization?  Because
doing so consumes extra CPU and results in extra cache misses, which is
the opposite of what you want on a completely busy system, especially
one where the CPUs are context switching quickly.  Thus no scan during
grace-period initialization.

But I can see the desire to share code.

One approach would be to embed the kernel_params_ops structure inside
another structure containing the limits, then just have two structures.
Perhaps something like this already exists?  I don't see it right off,
but then again, I am not exactly an expert on module_param.

Thoughts?

							Thanx, Paul

> Thank you in advance.
> Byungchul
> 
> ----->8-----
> >From 67fecc15b44b2521de96de109782c04ce65afb85 Mon Sep 17 00:00:00 2001
> From: Byungchul Park <byungchul.park@lge.com>
> Date: Tue, 29 May 2018 15:49:26 +0900
> Subject: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them
> 
> Currently, the range of jiffies_till_first_fqs and jiffies_till_next_fqs
> are always checked and adjusted in the loop of rcu_gp_kthread on runtime.
> However, it would be better and enough to check them only on setting
> them, so remove unnecessary checking and adjusting in the loop.
> 
> Additionally, add adjusting jiffies_till_first_fqs so guaranteed to be
> greater than 0, which hasn't been done before.
> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  kernel/rcu/tree.c | 38 +++++++++++++++++++++++++-------------
>  1 file changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 4e96761..4964237 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -518,8 +518,31 @@ void rcu_all_qs(void)
>  static ulong jiffies_till_next_fqs = ULONG_MAX;
>  static bool rcu_kick_kthreads;
> 
> -module_param(jiffies_till_first_fqs, ulong, 0644);
> -module_param(jiffies_till_next_fqs, ulong, 0644);
> +static int param_set_fqs_jiffies(const char *val, const struct kernel_param *kp)
> +{
> +	ulong tmp;
> +	int ret = kstrtoul(val, 0, &tmp);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	if (tmp > HZ)
> +		tmp = HZ;
> +	else if (tmp < 1)
> +		tmp = 1;
> +
> +	/* Prevent tearing */
> +	WRITE_ONCE(*(ulong *)kp->arg, tmp);
> +	return 0;
> +}
> +
> +static struct kernel_param_ops fqs_jiffies_ops = {
> +	.set = param_set_fqs_jiffies,
> +	.get = param_get_ulong,
> +};
> +
> +module_param_cb(jiffies_till_first_fqs, &fqs_jiffies_ops, &jiffies_till_first_fqs, 0644);
> +module_param_cb(jiffies_till_next_fqs, &fqs_jiffies_ops, &jiffies_till_next_fqs, 0644);
>  module_param(rcu_kick_kthreads, bool, 0644);
> 
>  /*
> @@ -2129,10 +2152,6 @@ static int __noreturn rcu_gp_kthread(void *arg)
>  		/* Handle quiescent-state forcing. */
>  		first_gp_fqs = true;
>  		j = jiffies_till_first_fqs;
> -		if (j > HZ) {
> -			j = HZ;
> -			jiffies_till_first_fqs = HZ;
> -		}
>  		ret = 0;
>  		for (;;) {
>  			if (!ret) {
> @@ -2167,13 +2186,6 @@ static int __noreturn rcu_gp_kthread(void *arg)
>  				WRITE_ONCE(rsp->gp_activity, jiffies);
>  				ret = 0; /* Force full wait till next FQS. */
>  				j = jiffies_till_next_fqs;
> -				if (j > HZ) {
> -					j = HZ;
> -					jiffies_till_next_fqs = HZ;
> -				} else if (j < 1) {
> -					j = 1;
> -					jiffies_till_next_fqs = 1;
> -				}
>  			} else {
>  				/* Deal with stray signal. */
>  				cond_resched_tasks_rcu_qs();
> -- 
> 1.9.1
> 

Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Byungchul Park]

On 2018-05-29 21:01, Paul E. McKenney wrote:
> On Tue, May 29, 2018 at 04:23:36PM +0900, Byungchul Park wrote:
>> Hello Paul and folks,
>>
>> I've thought the code should've been like the below since the range
>> checking of jiffies_till_first_fqs and jiffies_till_next_fqs everytime
>> in the loop of rcu_gp_kthread are unnecessary at all. However, it's ok
>> even if you don't think it's worth doing it.
> 
> Nice!
> 
>> Secondly, I also think jiffies_till_first_fqs = 0 is meaningless so
>> added checking and adjusting it as what's done on jiffies_till_next_fqs.
>> Thought?
> 
> Actually, jiffies_till_first_fqs == 0 is very useful for cases where
> at least one CPU is expected to be idle and grace-period latency is
> important.  In this case, doing the first scan immediately gets the
> dyntick-idle state recorded immediately, getting the idle CPUs out of
> the way of the grace period immediately.

Hi Paul~

You might want to handle it through sysfs. Otherwise, we can do it with
force_quiescent_state() IMHO.

> So why not do this scan as part of grace-period initialization?  Because
> doing so consumes extra CPU and results in extra cache misses, which is
> the opposite of what you want on a completely busy system, especially
> one where the CPUs are context switching quickly.  Thus no scan during
> grace-period initialization.

I am sorry I don't understand this paragraph. :(

> But I can see the desire to share code.
> 
> One approach would be to embed the kernel_params_ops structure inside
> another structure containing the limits, then just have two structures.
> Perhaps something like this already exists?  I don't see it right off,
> but then again, I am not exactly an expert on module_param.

It would be much nicer if we can as you said. I will check it.

Thanks a lot Paul.

-- 
Thanks,
Byungchul

Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Paul E. McKenney]

On Wed, May 30, 2018 at 10:06:52PM +0900, Byungchul Park wrote:
> 
> 
> On 2018-05-29 21:01, Paul E. McKenney wrote:
> >On Tue, May 29, 2018 at 04:23:36PM +0900, Byungchul Park wrote:
> >>Hello Paul and folks,
> >>
> >>I've thought the code should've been like the below since the range
> >>checking of jiffies_till_first_fqs and jiffies_till_next_fqs everytime
> >>in the loop of rcu_gp_kthread are unnecessary at all. However, it's ok
> >>even if you don't think it's worth doing it.
> >
> >Nice!
> >
> >>Secondly, I also think jiffies_till_first_fqs = 0 is meaningless so
> >>added checking and adjusting it as what's done on jiffies_till_next_fqs.
> >>Thought?
> >
> >Actually, jiffies_till_first_fqs == 0 is very useful for cases where
> >at least one CPU is expected to be idle and grace-period latency is
> >important.  In this case, doing the first scan immediately gets the
> >dyntick-idle state recorded immediately, getting the idle CPUs out of
> >the way of the grace period immediately.
> 
> Hi Paul~
> 
> You might want to handle it through sysfs. Otherwise, we can do it with
> force_quiescent_state() IMHO.

I agree that sysfs would be better than debugfs because these parameters
are about tuning, not debugging, so good point!

> >So why not do this scan as part of grace-period initialization?  Because
> >doing so consumes extra CPU and results in extra cache misses, which is
> >the opposite of what you want on a completely busy system, especially
> >one where the CPUs are context switching quickly.  Thus no scan during
> >grace-period initialization.
> 
> I am sorry I don't understand this paragraph. :(

Let me try again.  ;-)

I could change RCU to avoid the need for jiffies_till_first_fqs == 0,
but doing that would increase CPU consumption for workloads that are
already bottlenecked on the CPU.  So I won't be making that change,
so we still need jiffies_till_first_fqs == 0.

> >But I can see the desire to share code.
> >
> >One approach would be to embed the kernel_params_ops structure inside
> >another structure containing the limits, then just have two structures.
> >Perhaps something like this already exists?  I don't see it right off,
> >but then again, I am not exactly an expert on module_param.
> 
> It would be much nicer if we can as you said. I will check it.

Sounds very good!

							Thanx, Paul

> Thanks a lot Paul.
> 
> -- 
> Thanks,
> Byungchul
> 

Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Byungchul Park]

On 2018-05-30 22:49, Paul E. McKenney wrote:
> On Wed, May 30, 2018 at 10:06:52PM +0900, Byungchul Park wrote:
>>
>>
>> On 2018-05-29 21:01, Paul E. McKenney wrote:
>>> On Tue, May 29, 2018 at 04:23:36PM +0900, Byungchul Park wrote:
>>>> Hello Paul and folks,
>>>>
>>>> I've thought the code should've been like the below since the range
>>>> checking of jiffies_till_first_fqs and jiffies_till_next_fqs everytime
>>>> in the loop of rcu_gp_kthread are unnecessary at all. However, it's ok
>>>> even if you don't think it's worth doing it.
>>>
>>> Nice!
>>>
>>>> Secondly, I also think jiffies_till_first_fqs = 0 is meaningless so
>>>> added checking and adjusting it as what's done on jiffies_till_next_fqs.
>>>> Thought?
>>>
>>> Actually, jiffies_till_first_fqs == 0 is very useful for cases where
>>> at least one CPU is expected to be idle and grace-period latency is
>>> important.  In this case, doing the first scan immediately gets the
>>> dyntick-idle state recorded immediately, getting the idle CPUs out of
>>> the way of the grace period immediately.
>>
>> Hi Paul~
>>
>> You might want to handle it through sysfs. Otherwise, we can do it with
>> force_quiescent_state() IMHO.
> 
> I agree that sysfs would be better than debugfs because these parameters
> are about tuning, not debugging, so good point!
> 
>>> So why not do this scan as part of grace-period initialization?  Because
>>> doing so consumes extra CPU and results in extra cache misses, which is
>>> the opposite of what you want on a completely busy system, especially
>>> one where the CPUs are context switching quickly.  Thus no scan during
>>> grace-period initialization.
>>
>> I am sorry I don't understand this paragraph. :(
> 
> Let me try again.  ;-)
> 
> I could change RCU to avoid the need for jiffies_till_first_fqs == 0,
> but doing that would increase CPU consumption for workloads that are
> already bottlenecked on the CPU.  So I won't be making that change,
> so we still need jiffies_till_first_fqs == 0.

Thanks. I see. We need it then.

-- 
Thanks,
Byungchul

Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Byungchul Park]

On 2018-05-29 21:01, Paul E. McKenney wrote:

> One approach would be to embed the kernel_params_ops structure inside
> another structure containing the limits, then just have two structures.
> Perhaps something like this already exists?  I don't see it right off,
> but then again, I am not exactly an expert on module_param.
> 
> Thoughts?

Unfortunately, I couldn't find it. There might be no way to verify
range of a variable except the way I did. Could you give your opinion
about whether I should go on it?

-- 
Thanks,
Byungchul

Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Byungchul Park]

On 2018-05-31 11:18, Byungchul Park wrote:
> On 2018-05-29 21:01, Paul E. McKenney wrote:
> 
>> One approach would be to embed the kernel_params_ops structure inside
>> another structure containing the limits, then just have two structures.
>> Perhaps something like this already exists?  I don't see it right off,
>> but then again, I am not exactly an expert on module_param.
>>
>> Thoughts?
> 
> Unfortunately, I couldn't find it. There might be no way to verify
> range of a variable except the way I did. Could you give your opinion
> about whether I should go on it?

Like..

----->8-----
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 4e96761..eb54d7d 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -518,8 +518,38 @@ void rcu_all_qs(void)
  static ulong jiffies_till_next_fqs = ULONG_MAX;
  static bool rcu_kick_kthreads;

-module_param(jiffies_till_first_fqs, ulong, 0644);
-module_param(jiffies_till_next_fqs, ulong, 0644);
+static int param_set_first_fqs_jiffies(const char *val, const struct 
kernel_param *kp)
+{
+       ulong j;
+       int ret = kstrtoul(val, 0, &j);
+
+       if (!ret)
+               WRITE_ONCE(*(ulong *)kp->arg, (j > HZ) ? HZ : j);
+       return ret;
+}
+
+static int param_set_next_fqs_jiffies(const char *val, const struct 
kernel_param *kp)
+{
+       ulong j;
+       int ret = kstrtoul(val, 0, &j);
+
+       if (!ret)
+               WRITE_ONCE(*(ulong *)kp->arg, (j > HZ) ? HZ : (j ?: 1));
+       return ret;
+}
+
+static struct kernel_param_ops first_fqs_jiffies_ops = {
+       .set = param_set_first_fqs_jiffies,
+       .get = param_get_ulong,
+};
+
+static struct kernel_param_ops next_fqs_jiffies_ops = {
+       .set = param_set_next_fqs_jiffies,
+       .get = param_get_ulong,
+};
+
+module_param_cb(jiffies_till_first_fqs, &first_fqs_jiffies_ops, 
&jiffies_till_first_fqs, 0644);
+module_param_cb(jiffies_till_next_fqs, &next_fqs_jiffies_ops, 
&jiffies_till_next_fqs, 0644);
  module_param(rcu_kick_kthreads, bool, 0644);

  /*
@@ -2129,10 +2159,6 @@ static int __noreturn rcu_gp_kthread(void *arg)
                 /* Handle quiescent-state forcing. */
                 first_gp_fqs = true;
                 j = jiffies_till_first_fqs;
-               if (j > HZ) {
-                       j = HZ;
-                       jiffies_till_first_fqs = HZ;
-               }
                 ret = 0;
                 for (;;) {
                         if (!ret) {
@@ -2167,13 +2193,6 @@ static int __noreturn rcu_gp_kthread(void *arg)
                                 WRITE_ONCE(rsp->gp_activity, jiffies);
                                 ret = 0; /* Force full wait till next 
FQS. */
                                 j = jiffies_till_next_fqs;
-                               if (j > HZ) {
-                                       j = HZ;
-                                       jiffies_till_next_fqs = HZ;
-                               } else if (j < 1) {
-                                       j = 1;
-                                       jiffies_till_next_fqs = 1;
-                               }
                         } else {
                                 /* Deal with stray signal. */
                                 cond_resched_tasks_rcu_qs();


-- 
Thanks,
Byungchul

Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Paul E. McKenney]

On Thu, May 31, 2018 at 11:51:40AM +0900, Byungchul Park wrote:
> On 2018-05-31 11:18, Byungchul Park wrote:
> >On 2018-05-29 21:01, Paul E. McKenney wrote:
> >
> >>One approach would be to embed the kernel_params_ops structure inside
> >>another structure containing the limits, then just have two structures.
> >>Perhaps something like this already exists?  I don't see it right off,
> >>but then again, I am not exactly an expert on module_param.
> >>
> >>Thoughts?
> >
> >Unfortunately, I couldn't find it. There might be no way to verify
> >range of a variable except the way I did. Could you give your opinion
> >about whether I should go on it?
> 
> Like..

This looks reasonable to me.  Although you could make something that took
ranges, that would be more code than what you have below, so what you
have below is good.  Could you please resend as a patch with Signed-off-by
and commit log?

							Thanx, Paul

> ----->8-----
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 4e96761..eb54d7d 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -518,8 +518,38 @@ void rcu_all_qs(void)
>  static ulong jiffies_till_next_fqs = ULONG_MAX;
>  static bool rcu_kick_kthreads;
> 
> -module_param(jiffies_till_first_fqs, ulong, 0644);
> -module_param(jiffies_till_next_fqs, ulong, 0644);
> +static int param_set_first_fqs_jiffies(const char *val, const
> struct kernel_param *kp)
> +{
> +       ulong j;
> +       int ret = kstrtoul(val, 0, &j);
> +
> +       if (!ret)
> +               WRITE_ONCE(*(ulong *)kp->arg, (j > HZ) ? HZ : j);
> +       return ret;
> +}
> +
> +static int param_set_next_fqs_jiffies(const char *val, const struct
> kernel_param *kp)
> +{
> +       ulong j;
> +       int ret = kstrtoul(val, 0, &j);
> +
> +       if (!ret)
> +               WRITE_ONCE(*(ulong *)kp->arg, (j > HZ) ? HZ : (j ?: 1));
> +       return ret;
> +}
> +
> +static struct kernel_param_ops first_fqs_jiffies_ops = {
> +       .set = param_set_first_fqs_jiffies,
> +       .get = param_get_ulong,
> +};
> +
> +static struct kernel_param_ops next_fqs_jiffies_ops = {
> +       .set = param_set_next_fqs_jiffies,
> +       .get = param_get_ulong,
> +};
> +
> +module_param_cb(jiffies_till_first_fqs, &first_fqs_jiffies_ops,
> &jiffies_till_first_fqs, 0644);
> +module_param_cb(jiffies_till_next_fqs, &next_fqs_jiffies_ops,
> &jiffies_till_next_fqs, 0644);
>  module_param(rcu_kick_kthreads, bool, 0644);
> 
>  /*
> @@ -2129,10 +2159,6 @@ static int __noreturn rcu_gp_kthread(void *arg)
>                 /* Handle quiescent-state forcing. */
>                 first_gp_fqs = true;
>                 j = jiffies_till_first_fqs;
> -               if (j > HZ) {
> -                       j = HZ;
> -                       jiffies_till_first_fqs = HZ;
> -               }
>                 ret = 0;
>                 for (;;) {
>                         if (!ret) {
> @@ -2167,13 +2193,6 @@ static int __noreturn rcu_gp_kthread(void *arg)
>                                 WRITE_ONCE(rsp->gp_activity, jiffies);
>                                 ret = 0; /* Force full wait till
> next FQS. */
>                                 j = jiffies_till_next_fqs;
> -                               if (j > HZ) {
> -                                       j = HZ;
> -                                       jiffies_till_next_fqs = HZ;
> -                               } else if (j < 1) {
> -                                       j = 1;
> -                                       jiffies_till_next_fqs = 1;
> -                               }
>                         } else {
>                                 /* Deal with stray signal. */
>                                 cond_resched_tasks_rcu_qs();
> 
> 
> -- 
> Thanks,
> Byungchul
> 

Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them

[Byungchul Park]

On 2018-05-31 20:17, Paul E. McKenney wrote:
> On Thu, May 31, 2018 at 11:51:40AM +0900, Byungchul Park wrote:
>> On 2018-05-31 11:18, Byungchul Park wrote:
>>> On 2018-05-29 21:01, Paul E. McKenney wrote:
>>>
>>>> One approach would be to embed the kernel_params_ops structure inside
>>>> another structure containing the limits, then just have two structures.
>>>> Perhaps something like this already exists?  I don't see it right off,
>>>> but then again, I am not exactly an expert on module_param.
>>>>
>>>> Thoughts?
>>>
>>> Unfortunately, I couldn't find it. There might be no way to verify
>>> range of a variable except the way I did. Could you give your opinion
>>> about whether I should go on it?
>>
>> Like..
> 
> This looks reasonable to me.  Although you could make something that took
> ranges, that would be more code than what you have below, so what you

Exactly.

> have below is good.  Could you please resend as a patch with Signed-off-by
> and commit log?

Sure, I will. Thanks a lot Paul.

-- 
Thanks,
Byungchul