Messages in this thread |  | | From | Vitaly Kuznetsov <> | | Subject | Re: [PATCH 10/17] hyper-v: trace vmbus_open() | | Date | Mon, 30 Oct 2017 11:07:01 +0100 |
| |
Greg KH <gregkh@linuxfoundation.org> writes:
> On Mon, Oct 30, 2017 at 09:16:19AM +0100, Vitaly Kuznetsov wrote: >> Greg KH <gregkh@linuxfoundation.org> writes: >> >> > On Sun, Oct 29, 2017 at 12:21:09PM -0700, kys@exchange.microsoft.com wrote: >> >> From: Vitaly Kuznetsov <vkuznets@redhat.com> >> >> >> >> Add tracepoint to CHANNELMSG_OPENCHANNEL sender. >> >> >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> >> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> >> >> --- >> >> drivers/hv/channel.c | 2 ++ >> >> drivers/hv/hv_trace.h | 27 +++++++++++++++++++++++++++ >> >> 2 files changed, 29 insertions(+) >> >> >> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c >> >> index a406beb10dd0..739b3fe1e0fb 100644 >> >> --- a/drivers/hv/channel.c >> >> +++ b/drivers/hv/channel.c >> >> @@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size, >> >> ret = vmbus_post_msg(open_msg, >> >> sizeof(struct vmbus_channel_open_channel), true); >> >> >> >> + trace_vmbus_open(open_msg, ret); >> > >> > Why add tracepoints for things that ftrace can handle automatically? >> >> This series adds pretty prints for structures printing what is needed >> and in the right format significantly simplifying debugging. And it >> wouldn't make sense to add tracepoints to *some* messages-related >> functions and skip others where parsing is more trivial. > > Tracepoints add memory usage and take up real space. If you don't need > them for something, as there are other ways to already get the > information needed, why add new ones that you now need to drag around > for all time? >
Are you opposed to the series as a whole (AKA 'no tracepoints in drivers') or only to some tracepoints we add here?
Debugging stuff is always a tradeof between some memory overhead and convenience (and CONFIG_TRACEPOINTS is the knob). Here I'd like to see vmbus_* structures parsed in output. When things go wrong I can tell someone "do echo 1 > /sys/kernel/debug/tracing/events/hyperv/enable and show me the output" and I will have enough information to figure out what's going on.
I'm probably missing something about easy 'alternative' ways to get what I need and why people add tracepoints to drivers in general.
-- Vitaly
|  |