This thread is a little stale, and it looks like the PR is as well. Did anything more happen? I was pretty happy with the direction of work, although I haven’t fully assimilated this last email. -Greg On Tue, Jun 12, 2018 at 2:02 PM, Mohamad Gebai <mgebai@xxxxxxx> wrote: > > 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 -- 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
