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]
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/

Powered by blists - more mailing lists