Messages in this thread | | | From | Dmitry Rokosov <> | Subject | Re: [PATCH v1] regmap: introduce value tracing for regmap bulk operations | Date | Thu, 18 Aug 2022 17:02:46 +0000 |
| |
Hello Marc,
Thank you for quick feedback. Please find my comments below.
On Thu, Aug 18, 2022 at 02:49:20PM +0100, Marc Zyngier wrote:
[...]
> > > Currently, only one-register io operations support tracepoints with > > > value logging. For the regmap bulk operations developer can view > > > hw_start/hw_done tracepoints with starting reg number and registers > > > count to be reading or writing. This patch injects tracepoints with > > > dumping registers values in the hex format to regmap bulk reading > > > and writing. > > I don't care much about regmap as a MMIO backend, but it strikes me as > odd that you end up with multiple ways of logging the same stuff (with > a memcpy in the middle of it). > > Why can't this be done with a small amount of trace post-processing?
Sorry, actually, I don't get you. What do you mean by "same stuff"? For now, regmap bulk I/O operations don't log data buffers, because current regmap trace classes don't have a dynamic trace arrays inside. We should use dynamic array here because bulk I/O operations can vary buffer size from call to call. Function memcpy() is used to copy original buffer data to a trace array when tracepoint is enabled. In other words, per my understanding, when tracepoint is disabled we do not call TP_fast_assign instructions.
Trace event documentation says about dynamic array:
* __dynamic_array: This is similar to array, but can vary its size from * instance to instance of the tracepoint being called. * Like __array, this too has three elements (type, name, size); * type is the type of the element, name is the name of the array. * The size is different than __array. It is not a static number, * but the algorithm to figure out the length of the array for the * specific instance of tracepoint. Again, size is the number of * items in the array, not the total length in bytes. * * __dynamic_array( int, foo, bar) is similar to: int foo[bar]; * * Note, unlike arrays, you must use the __get_dynamic_array() macro * to access the array. * * memcpy(__get_dynamic_array(foo), bar, 10); * * Notice, that "__entry" is not needed here.
BTW, I've tried to use the already existing TRACE CLASS regmap_block, but it's difficult to integrate dynamic array to that, because sometimes we do not have a reg data (for example, regmap_hw_read_start event).
[...]
-- Thank you, Dmitry
| |