LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Byungchul Park <byungchul.park@lge.com>
Cc: jiangshanlai@gmail.com, josh@joshtriplett.org,
	rostedt@goodmis.org, mathieu.desnoyers@efficios.com,
	linux-kernel@vger.kernel.org, kernel-team@lge.com,
	joel@joelfernandes.org
Subject: Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them
Date: Thu, 31 May 2018 04:17:42 -0700	[thread overview]
Message-ID: <20180531111742.GQ7063@linux.vnet.ibm.com> (raw)
In-Reply-To: <3d2d8dec-445a-da31-dbe0-34b2331523ed@lge.com>

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
> 

  reply	other threads:[~2018-05-31 11:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-29  7:23 Byungchul Park
2018-05-29 12:01 ` Paul E. McKenney
2018-05-30 13:06   ` Byungchul Park
2018-05-30 13:49     ` Paul E. McKenney
2018-05-31  1:27       ` Byungchul Park
2018-05-31  2:18   ` Byungchul Park
2018-05-31  2:51     ` Byungchul Park
2018-05-31 11:17       ` Paul E. McKenney [this message]
2018-06-01  1:42         ` Byungchul Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180531111742.GQ7063@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=byungchul.park@lge.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=rostedt@goodmis.org \
    --subject='Re: [RFC] rcu: Check the range of jiffies_till_xxx_fqs on setting them' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).