Philip Guenther wrote:
> On Mon, Oct 26, 2015 at 8:46 AM, Michael McConville <mm...@mykolab.com> wrote:
> > We have a pretty strong guarantee that it can only happen once per
> > process...
> ...
> > --- sys/sys/syscall_mi.h        9 Oct 2015 01:17:18 -0000       1.11
> > +++ sys/sys/syscall_mi.h        26 Oct 2015 15:13:44 -0000
> > @@ -72,7 +72,8 @@ mi_syscall(struct proc *p, register_t co
> >         if (lock)
> >                 KERNEL_LOCK();
> >         pledged = (p->p_p->ps_flags & PS_PLEDGE);
> > -       if (pledged && !(tval = pledge_check(p, code))) {
> > +       if (__predict_false(
> > +           pledged && !(tval = pledge_check(p, code)))) {
> 
> I disagree.  That's the code used on every syscall, not just once per
> process and pledged is true for *most* of the processes on a -current
> box.  No, that doesn't mean we should do __predict_true() there.

That's what I mean - it's used on every syscall and it can never be true
more than once per process. (That's a pledge failure, and the program
will be terminated.)

> In general, __predict_{true,false} should be left in the tool box and
> only pulled out after detailed dives into code paths involved.  For
> all my banging on the project, I think I've used them in *two* places.

Passing thought: it'd be nice if the names were shorter. I feel like
much of the reason they're seen as cumbersome is that they're so
verbose. Linux uses likely() and unlikely(). __rare() could also work.

FWIW, below is what /usr/include/sys/cdefs.h says about them. Maybe
outdated.


/*
 * GNU C version 2.96 adds explicit branch prediction so that
 * the CPU back-end can hint the processor and also so that
 * code blocks can be reordered such that the predicted path
 * sees a more linear flow, thus improving cache behavior, etc.
 *
 * The following two macros provide us with a way to utilize this
 * compiler feature.  Use __predict_true() if you expect the expression
 * to evaluate to true, and __predict_false() if you expect the
 * expression to evaluate to false.
 *
 * A few notes about usage:
 *
 *      * Generally, __predict_false() error condition checks (unless
 *        you have some _strong_ reason to do otherwise, in which case
 *        document it), and/or __predict_true() `no-error' condition
 *        checks, assuming you want to optimize for the no-error case.
 *
 *      * Other than that, if you don't know the likelihood of a test
 *        succeeding from empirical or other `hard' evidence, don't
 *        make predictions.
 *
 *      * These are meant to be used in places that are run `a lot'.
 *        It is wasteful to make predictions in code that is run
 *        seldomly (e.g. at subsystem initialization time) as the
 *        basic block reordering that this affects can often generate
 *        larger code.
 */

Reply via email to