On Tue, Oct 19, 2021 at 7:28 PM Steven Rostedt rostedt@goodmis.org wrote:
On Tue, 19 Oct 2021 18:31:39 -0700 Kalesh Singh kaleshsingh@google.com wrote:
+static u64 hist_field_div(struct hist_field *hist_field,
struct tracing_map_elt *elt,struct trace_buffer *buffer,struct ring_buffer_event *rbe,void *event)+{
struct hist_field *operand1 = hist_field->operands[0];struct hist_field *operand2 = hist_field->operands[1];u64 val1 = operand1->fn(operand1, elt, buffer, rbe, event);u64 val2 = operand2->fn(operand2, elt, buffer, rbe, event);/* Return -1 for the undefined case */if (!val2)return -1;return div64_u64(val1, val2);+}
I wonder if you should add a shift operator as well?
I mean, if for some reason you want to divide by a power of two, then why us the division. Especially if this is on a 32 bit machine.
Of course, the parsing could detect that. If the divisor is a constant. Or we could even optimize the above with:
if (!val2) return -1; if (!(val2 & (val2 - 1)) return val1 >> __ffs64(val2);Which should be faster than a divide, and even if it isn't a power of two, the subtract and & should be in the noise compared to the divide.
Note, the above can be added to this. I'm not suggesting changing this patch.
Is it worth adding something like this for the multiplication case as well?
- Kalesh
-- Steve