On 06/07/2018 01:24 PM, Mohamad Gebai wrote: > > On 06/07/2018 12:52 PM, Sage Weil wrote: >> On Thu, 7 Jun 2018, Mohamad Gebai wrote: >>> [snip] >>> Any thought about the approach? How can I move this forward? >> This is a huge improvement over manually writing the .tp files! I have >> one thought, though. It's still necessary both to adjust the tracepoint >> location, e.g., >> >> >> - dout(30) << __func__ << " " << *(b_it->first) >> - << " expected4release=" << blob_expected_for_release >> - << " expected_allocations=" << bi.expected_allocations >> - << dendl; >> + trace_process_protrusive_extents_expected4release(*(b_it->first), >> + blob_expected_for_release, bi.expected_allocations); >> >> and also to include the line in tracing/tracetool/subsys, eg., >> >> + process_protrusive_extents_expected4release(Blob blob, int64_t blob_expected_for_release, int64_t expected_allocations) "%s expected4release=%llu expected_allocations=%llu" 30 >> >> Do you think it's possible to declare these inline and generate the >> tracing/tracetool/subsys files from the source? For example, write >> something like >> >> trace(30, process_protrusive_extents_expected4release, >> Blob *(b_it->first), >> int64_t blob_expected_for_release, >> int64_t bi.expected_allocations, >> "%s expected4release=%llu expected_allocations=%llu"); >> >> A macro could expand that out to the function call, and a preprocessor >> pass could slurp these up and generate the file for input to >> tracetool (or tracetool could extract them from the source >> directly). > Good point, I definitely see that as a possibility. I'll fiddle with it > and report back. Ok, I think I got to something that is feasible and hopefully less awkward. Consider the following syntax in the call site: trace_<event_name>(<loglevel>, <subsys>, <type1>, <name1>, <value1> [, <type2>, <name2>, <value2>, ...], <format_string>); When expanded, this macro takes everything except the values out, as such: __trace_<event_name>(<value1> [, <value2>, ...]); The example you gave goes from: dout(30) << __func__ << " " << *(b_it->first) << " expected4release=" << blob_expected_for_release << " expected_allocations=" << bi.expected_allocations << dendl; to: trace_process_protrusive_extents_expected4release(30, bluestore_gc, Blob, blob, *(b_it->first), int64_t, blob_expected_for_release, blob_expected_for_release, int64_t, expected_allocations, bi.expected_allocations, "%s expected4release=%llu expected_allocations=%llu"); which when expanded becomes: __trace_process_protrusive_extents_expected4release( *(b_it->first), blob_expected_for_release, bi.expected_allocations, ); We'll need not only to specify the type of each field, but also the name we want it to have. Otherwise, what would be the name of the first argument of this example (*(b_it->first))? The good thing now is that this syntax is self-declarative, and I'm generating valid code (and tp files) from it. The bad thing is that it's not straightforward. There is also the "parsing the entire code base" part.. Thoughts? Is that something we can consider? PS: I haven't updated the PR yet. Mohamad -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
