Messages in this thread | | | From | Kalesh Singh <> | Date | Fri, 29 Oct 2021 09:39:59 -0700 | Subject | Re: [PATCH] tracing/histogram: Optimize division by constants |
| |
On Thu, Oct 28, 2021 at 7:01 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Wed, 27 Oct 2021 20:28:54 -0700 > Kalesh Singh <kaleshsingh@google.com> wrote: > > > +/* > > + * Returns the specific division function to use if the divisor > > + * is constant. This avoids extra branches when the trigger is hit. > > + */ > > +static hist_field_fn_t hist_field_get_div_fn(struct hist_field *divisor) > > +{ > > + u64 div; > > + > > + if (divisor->flags & HIST_FIELD_FL_VAR_REF) { > > + struct hist_field *var; > > + > > + var = find_var_field(divisor->var.hist_data, divisor->name); > > + div = var->constant; > > + } else > > + div = divisor->constant; > > + > > + if (!div) > > + return div_by_zero; > > Do we really need a div_by_zero constant function? What about just > erroring here and perhaps return -EDOM?
Good point. We can detect this if the divisor is a constant and print an error message instead. It'll also warrant a small update to the tests and documentation. I'll send out a new version addressing these.
Thanks, Kalesh
> > -- Steve > > > + > > + if (!(div & (div - 1))) > > + return div_by_power_of_two; > > + > > + /* If the divisor is too large, do a regular division */ > > + if (div > (1 << HIST_DIV_SHIFT)) > > + return div_by_not_power_of_two; > > + > > + divisor->div_multiplier = div64_u64((u64)(1 << HIST_DIV_SHIFT), div); > > + return div_by_mult_and_shift; > > +} > > +
| |