Messages in this thread | | | Date | Thu, 1 Dec 2011 16:13:50 -0500 | From | Jason Baron <> | Subject | Re: [PATCH] jump_label: jump_label for boot options. |
| |
On Thu, Dec 01, 2011 at 06:39:45PM +0100, Peter Zijlstra wrote: > On Thu, 2011-12-01 at 11:50 -0500, Jason Baron wrote: > > > I think its just a matter of reversing the true and false returns. > > That is, instead of: > > that's the same as !static_branch() > > > jump_label_inc/dec(), don't need to be changed, they just mean reverse > > the branch on 0, 1 transitions. Although using the same key in both > > static_branch_true, and static_branch_false, might be confusing. Maybe > > we rename jump_label_inc/dec to static_branch_reverse_inc()/dec()? > > Right, that's the problem really. That makes it impossible to make the > control code generic. > > What I'd want is something that doesn't out-of-line the branch, is > possibly enabled by default, but has the same inc/dec behaviour, not the > reversed. >
I think what you have below should work modulo the no out-of-line branches and the following change:
> - if (neg) > + if (neg) { > sysctl_sched_features &= ~(1UL << i); > - else > +#ifdef HAVE_JUMP_LABEL > + if (!jump_label_enabled(&sched_feat_keys[i])) > + jump_label_inc(&sched_feat_keys[i]); > +#endif
I think here its: if (jump_label_enabled()) jump_label_dec();
> + } else { > sysctl_sched_features |= (1UL << i); > +#ifdef HAVE_JUMP_LABEL > + if (jump_label_enabled(&sched_feat_keys[i])) > + jump_label_dec(&sched_feat_keys[i]); > +#endif > + }
Same here: if (!jump_label_enabled()) jump_label_inc()
The inc/dec behavior we have now, in fact will only mess up in the case where we define 'static_branch_true()'. Because then, in that case the jump_label_inc() will cause a jump to the false branch. So as long as we don't introduce 'static_branch_true()' and do an early setting of those branches which are true via __init code as you have here, I think things are correct.
Thanks,
-Jason
| |