Messages in this thread | | | Date | Wed, 20 Aug 2014 23:20:18 +0900 | Subject | Re: [PATCH 2/2] fault-inject: add ratelimit option | From | Akinobu Mita <> |
| |
2014-08-20 21:54 GMT+09:00 Dmitry Monakhov <dmonakhov@openvz.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.
| |