Re: [PATCH 2/2] fault-inject: add ratelimit option

[Akinobu Mita]

2014-08-20 21:54 GMT+09:00 Dmitry Monakhov <dmonakhov@xxxxxxxxxx>:
> @@ -40,10 +40,25 @@ EXPORT_SYMBOL_GPL(setup_fault_attr);
>
>  static void fail_dump(struct fault_attr *attr)
>  {
> -       if (attr->verbose > 0)
> -               printk(KERN_NOTICE "FAULT_INJECTION: forcing a failure\n");
> -       if (attr->verbose > 1)
> -               dump_stack();
> +       if (attr->verbose > 0 &&
> +           ___ratelimit(&attr->ratelimit_state, __func__)) {

You can use __ratelimit(state) instead of ___ratelimit(state, __func__).

> +               if (attr->dname)
> +                       printk(KERN_NOTICE "FAULT_INJECTION: forcing a failure."
> +                              "name %pd, interval %lu, probability %lu, "
> +                              "space %d, times %d\n", attr->dname,
> +                              attr->probability, attr->interval,
> +                              atomic_read(&attr->space),
> +                              atomic_read(&attr->times));
> +               else
> +                       printk(KERN_NOTICE "FAULT_INJECTION: forcing a "
> +                              "failure, interval %lu, probability %lu, "
> +                              "space %d, times %d\n", attr->probability,
> +                              attr->interval, atomic_read(&attr->space),
> +                              atomic_read(&attr->times));

This looks a bit redundant.  NULL pointer to "%pd" format produces
"(null)" string, so this printk and if-else can be removed.

Also, this message line is a bit longer than usual kernel message.
Should we put '\n' after "forcing a failure."?

This patch looks good to me.  Please feel free to add my ACK.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/