On Thu, 20 Jun 2024 00:48:55 +0200 Vlastimil Babka <vbabka@xxxxxxx> wrote: > +static int debugfs_prob_set(void *data, u64 val) > +{ > + struct fault_attr *attr = data; > + > + mutex_lock(&probability_mutex); > + > + if (attr->active) { > + if (attr->probability != 0 && val == 0) { > + static_key_slow_dec(attr->active); > + } else if (attr->probability == 0 && val != 0) { > + static_key_slow_inc(attr->active); > + } > + } So basically the above is testing if val to probability is going from zero or non-zero. For such cases, I find it less confusing to have: if (attr->active) { if (!!attr->probability != !!val) { if (val) static_key_slow_inc(attr->active); else static_key_slow_dec(attr->active); } } This does add a layer of nested ifs, but IMO it's a bit more clear in what is happening, and it gets rid of the "else if". Not saying you need to change it. This is more of an FYI. -- Steve > + > + attr->probability = val; > + > + mutex_unlock(&probability_mutex); > + > + return 0; > +}