For those interested, I've updated the PR, and the README in it. Please see the latest comment for some additional info. Mohamad On 07/06/2018 10:27 PM, Mohamad Gebai wrote: > Hi Greg, > > You're right, it got a little stale, sorry about that. It definitely did > *not* get side-tracked, though. I have the next iteration ready, minus > code cleaning which I haven't found time to do this past week. I will > update the PR early next week and will probably ask for a volunteer to > try it (plus the code review) to address the usability within Ceph. > > The gist of that last email I sent was that, following Sage's > suggestions, I got rid of having to define tracepoints in some external > text file before using them. > > Mohamad > > > On 07/06/2018 01:46 PM, Gregory Farnum wrote: >> 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 -- 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
