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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAC5umyjaE6HimXyBc52jMvGYM3OgQCX1aOhQxtZ90j+9vfGnSg@mail.gmail.com>
Date:	Wed, 20 Aug 2014 23:20:18 +0900
From:	Akinobu Mita <akinobu.mita@...il.com>
To:	Dmitry Monakhov <dmonakhov@...nvz.org>
Cc:	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] fault-inject: add ratelimit option

2014-08-20 21:54 GMT+09:00 Dmitry Monakhov <dmonakhov@...nvz.org>:
> @@ -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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/