Hi Andy, thanks for the review,
On Tue, 2018-06-26 at 20:04 +0300, Andy Shevchenko wrote [..]
#define RATELIMIT_STATE_INIT(name, interval_init, burst_init) { \
.lock =__RAW_SPIN_LOCK_UNLOCKED(name.lock), \
name is now redundant, isn't it?
It is. Worth to split on the second patch or keep callers changes in this patch?
@@ -42,9 +41,10 @@ static inline void ratelimit_state_init(struct ratelimit_state *rs, { memset(rs, 0, sizeof(*rs));
raw_spin_lock_init(&rs->lock); rs->interval = interval; rs->burst = burst;
atomic_set(&rs->printed, 0);atomic_set(&rs->missed, 0);Can it be
*rs = RATELIMIT_STATE_INIT(interval, burst);
?
(Yes, the '(struct ratelimit_state)' has to be added to macro to allow this)
Sure.
static inline void ratelimit_state_exit(struct ratelimit_state *rs) {
int missed;if (!(rs->flags & RATELIMIT_MSG_ON_RELEASE)) return;
if (rs->missed) {
if ((missed = atomic_xchg(&rs->missed, 0)))Perhaps
missed = ... if (missed)
?
Ok, will change - checkpatch has warned me, but I thought it's just a preference than a rule.
pr_warn("%s: %d output lines suppressed due toratelimiting\n",
current->comm, rs->missed);rs->missed = 0;}
current->comm, missed);} +static void ratelimit_end_interval(struct ratelimit_state *rs, const char *func) +{
rs->begin = jiffies;if (!(rs->flags & RATELIMIT_MSG_ON_RELEASE)) {unsigned missed = (unsigned)atomic_xchg(&rs-missed, 0);
if (missed)pr_warn("%s: %u callbacks suppressed\n",func, missed);
Instead of casting, perhaps
int missed = ...
I think you already has a guard against going it below zero. Or I missed something?
No, I do: atomic_add_unless(&rs->missed, 1, -1);
So, it's guard against overflow, but not against negative. That's why I do print it as unsigned.