Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Wed, Jan 22, 2020 at 10:39:15AM -0500, Sean Paul wrote: > On Wed, Jan 22, 2020 at 3:06 AM Daniel Vetter wrote: > > > > On Mon, Jan 20, 2020 at 01:56:21PM -0500, Steven Rostedt wrote: > > > On Thu, 16 Jan 2020 07:27:22 +0100 > > > Daniel Vetter wrote: > > > > > > > On Tue, Jan 14, 2020 at 12:21:43PM -0500, Sean Paul wrote: > > > > > From: Sean Paul > > > > > > > > > > This patch uses a ring_buffer to keep a "flight recorder" (name > > > > > credit Weston) > > > > > of DRM logs for a specified set of debug categories. The user writes a > > > > > bitmask of debug categories to the "trace_mask" node and can read log > > > > > messages from the "trace" node. > > > > > > > > > > These nodes currently exist in debugfs under the dri directory. I > > > > > intended on exposing all of this through tracefs originally, but the > > > > > tracefs entry points are not exposed, so there's no way to create > > > > > tracefs files from drivers at the moment. I think it would be a > > > > > worthwhile endeavour, but one requiring more time and conversation to > > > > > ensure the drm traces fit somewhere sensible. > > > > > > > > Hm, since the idea is to ship this in production environments debugfs is > > > > out. sysfs is also kinda the wrong thing, so maybe trying to get this > > > > stuffed into tracefs is actually the way to go? > > > > > > > > > > Why not use normal tracepoints and the tracing infrastructure? You can > > > add your own instance as rasdaemon does, which isn't affected by other > > > tracing. There's code now to even create these instances and enable and > > > disable events from within the kernel. > > > > > > > > > https://lore.kernel.org/lkml/1574276919-9-1-git-send-email-divya.i...@oracle.com/ > > > > Hm, without looking at the details this indeed seems like the thing we > > want ... Sean? > > Ohh indeed, I think we could make this work. Thanks for the pointer, Steven! > > The only item that needs sorting is: how does userspace select which > debug events are traced. I think we could solve both with another > module parameter to sit beside drm.debug with the same semantics (call > it drm.trace)? Yeah if there's no official way (in tracefs) for this drm.trace sounds reasonable. -Daniel > > > Sean > > > -Daniel > > > > > > > > As this is tracefs, you can mount it without even compiling in debugfs. > > > > > > -- Steve > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Wed, Jan 22, 2020 at 3:06 AM Daniel Vetter wrote: > > On Mon, Jan 20, 2020 at 01:56:21PM -0500, Steven Rostedt wrote: > > On Thu, 16 Jan 2020 07:27:22 +0100 > > Daniel Vetter wrote: > > > > > On Tue, Jan 14, 2020 at 12:21:43PM -0500, Sean Paul wrote: > > > > From: Sean Paul > > > > > > > > This patch uses a ring_buffer to keep a "flight recorder" (name credit > > > > Weston) > > > > of DRM logs for a specified set of debug categories. The user writes a > > > > bitmask of debug categories to the "trace_mask" node and can read log > > > > messages from the "trace" node. > > > > > > > > These nodes currently exist in debugfs under the dri directory. I > > > > intended on exposing all of this through tracefs originally, but the > > > > tracefs entry points are not exposed, so there's no way to create > > > > tracefs files from drivers at the moment. I think it would be a > > > > worthwhile endeavour, but one requiring more time and conversation to > > > > ensure the drm traces fit somewhere sensible. > > > > > > Hm, since the idea is to ship this in production environments debugfs is > > > out. sysfs is also kinda the wrong thing, so maybe trying to get this > > > stuffed into tracefs is actually the way to go? > > > > > > > Why not use normal tracepoints and the tracing infrastructure? You can > > add your own instance as rasdaemon does, which isn't affected by other > > tracing. There's code now to even create these instances and enable and > > disable events from within the kernel. > > > > > > https://lore.kernel.org/lkml/1574276919-9-1-git-send-email-divya.i...@oracle.com/ > > Hm, without looking at the details this indeed seems like the thing we > want ... Sean? Ohh indeed, I think we could make this work. Thanks for the pointer, Steven! The only item that needs sorting is: how does userspace select which debug events are traced. I think we could solve both with another module parameter to sit beside drm.debug with the same semantics (call it drm.trace)? Sean > -Daniel > > > > > As this is tracefs, you can mount it without even compiling in debugfs. > > > > -- Steve > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Mon, Jan 20, 2020 at 01:56:21PM -0500, Steven Rostedt wrote: > On Thu, 16 Jan 2020 07:27:22 +0100 > Daniel Vetter wrote: > > > On Tue, Jan 14, 2020 at 12:21:43PM -0500, Sean Paul wrote: > > > From: Sean Paul > > > > > > This patch uses a ring_buffer to keep a "flight recorder" (name credit > > > Weston) > > > of DRM logs for a specified set of debug categories. The user writes a > > > bitmask of debug categories to the "trace_mask" node and can read log > > > messages from the "trace" node. > > > > > > These nodes currently exist in debugfs under the dri directory. I > > > intended on exposing all of this through tracefs originally, but the > > > tracefs entry points are not exposed, so there's no way to create > > > tracefs files from drivers at the moment. I think it would be a > > > worthwhile endeavour, but one requiring more time and conversation to > > > ensure the drm traces fit somewhere sensible. > > > > Hm, since the idea is to ship this in production environments debugfs is > > out. sysfs is also kinda the wrong thing, so maybe trying to get this > > stuffed into tracefs is actually the way to go? > > > > Why not use normal tracepoints and the tracing infrastructure? You can > add your own instance as rasdaemon does, which isn't affected by other > tracing. There's code now to even create these instances and enable and > disable events from within the kernel. > > > https://lore.kernel.org/lkml/1574276919-9-1-git-send-email-divya.i...@oracle.com/ Hm, without looking at the details this indeed seems like the thing we want ... Sean? -Daniel > > As this is tracefs, you can mount it without even compiling in debugfs. > > -- Steve -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Thu, 16 Jan 2020 07:27:22 +0100 Daniel Vetter wrote: > On Tue, Jan 14, 2020 at 12:21:43PM -0500, Sean Paul wrote: > > From: Sean Paul > > > > This patch uses a ring_buffer to keep a "flight recorder" (name credit > > Weston) > > of DRM logs for a specified set of debug categories. The user writes a > > bitmask of debug categories to the "trace_mask" node and can read log > > messages from the "trace" node. > > > > These nodes currently exist in debugfs under the dri directory. I > > intended on exposing all of this through tracefs originally, but the > > tracefs entry points are not exposed, so there's no way to create > > tracefs files from drivers at the moment. I think it would be a > > worthwhile endeavour, but one requiring more time and conversation to > > ensure the drm traces fit somewhere sensible. > > Hm, since the idea is to ship this in production environments debugfs is > out. sysfs is also kinda the wrong thing, so maybe trying to get this > stuffed into tracefs is actually the way to go? > Why not use normal tracepoints and the tracing infrastructure? You can add your own instance as rasdaemon does, which isn't affected by other tracing. There's code now to even create these instances and enable and disable events from within the kernel. https://lore.kernel.org/lkml/1574276919-9-1-git-send-email-divya.i...@oracle.com/ As this is tracefs, you can mount it without even compiling in debugfs. -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Tue, Jan 14, 2020 at 12:21:43PM -0500, Sean Paul wrote:
> From: Sean Paul
>
> This patch uses a ring_buffer to keep a "flight recorder" (name credit Weston)
> of DRM logs for a specified set of debug categories. The user writes a
> bitmask of debug categories to the "trace_mask" node and can read log
> messages from the "trace" node.
>
> These nodes currently exist in debugfs under the dri directory. I
> intended on exposing all of this through tracefs originally, but the
> tracefs entry points are not exposed, so there's no way to create
> tracefs files from drivers at the moment. I think it would be a
> worthwhile endeavour, but one requiring more time and conversation to
> ensure the drm traces fit somewhere sensible.
Hm, since the idea is to ship this in production environments debugfs is
out. sysfs is also kinda the wrong thing, so maybe trying to get this
stuffed into tracefs is actually the way to go?
-Daniel
>
> Cc: Daniel Vetter
> Cc: David Airlie
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Pekka Paalanen
> Cc: Rob Clark
> Cc: Steven Rostedt
> Cc: Thomas Zimmermann
> Cc: Ville Syrjälä
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-s...@poorly.run
> #v1
> Link:
> https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-s...@poorly.run
> #v3
>
> Changes in v2:
> - Went with a completely different approach:
> https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html
>
> Changes in v3:
> - Changed commit message to be a bit less RFC-y
> - Make class_drm_category_log an actual trace class
>
> Changes in v4:
> - Instead of [ab]using trace events and the system trace buffer, use our
> own ringbuffer
> ---
> ---
> Documentation/gpu/drm-uapi.rst | 9 +
> drivers/gpu/drm/Kconfig| 1 +
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_drv.c | 3 +
> drivers/gpu/drm/drm_print.c| 80 +--
> drivers/gpu/drm/drm_trace.c| 376 +
> include/drm/drm_print.h| 39
> 7 files changed, 487 insertions(+), 23 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_trace.c
>
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 56fec6ed1ad8..089eb6fd3e94 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -312,6 +312,15 @@ Debugfs Support
> .. kernel-doc:: drivers/gpu/drm/drm_debugfs.c
> :export:
>
> +DRM Tracing
> +---
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_trace.c
> + :doc: DRM Tracing
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_trace.c
> + :internal:
> +
> Sysfs Support
> =
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index d0aa6cff2e02..9d8077e87afe 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -14,6 +14,7 @@ menuconfig DRM
> select I2C
> select I2C_ALGOBIT
> select DMA_SHARED_BUFFER
> + select RING_BUFFER
> select SYNC_FILE
> help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6493088a0fdd..88b4674934e6 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -29,7 +29,7 @@ drm-$(CONFIG_DRM_PANEL) += drm_panel.o
> drm-$(CONFIG_OF) += drm_of.o
> drm-$(CONFIG_AGP) += drm_agpsupport.o
> drm-$(CONFIG_PCI) += drm_pci.o
> -drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
> +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o drm_trace.o
> drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>
> drm_vram_helper-y := drm_gem_vram_helper.o \
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7c18a980cd4b..98260b9f8004 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1114,6 +1114,7 @@ static const struct file_operations drm_stub_fops = {
> static void drm_core_exit(void)
> {
> unregister_chrdev(DRM_MAJOR, "drm");
> + drm_trace_cleanup();
> debugfs_remove(drm_debugfs_root);
> drm_sysfs_destroy();
> idr_destroy(&drm_minors_idr);
> @@ -1135,6 +1136,8 @@ static int __init drm_core_init(void)
>
> drm_debugfs_root = debugfs_create_dir("dri", NULL);
>
> + WARN_ON(drm_trace_init(drm_debugfs_root));
> +
> ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
> if (ret < 0)
> goto error;
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 111b932cf2a9..0ac1867937bf 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -262,21 +262,37 @@ void drm_dev_dbg(const struct device *dev, enum
> drm_debug_category category,
> struct va_format vaf;
> va_list args;
>
> - if (!drm_debug_enabled(category)
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Wed, Jan 15, 2020 at 05:38:13PM +, Chris Wilson wrote: > Quoting Sean Paul (2020-01-15 14:21:18) > > On Wed, Jan 15, 2020 at 02:01:19PM +, Chris Wilson wrote: > > > Quoting Sean Paul (2020-01-15 13:41:58) > > > > On Wed, Jan 15, 2020 at 10:36:36AM +, Chris Wilson wrote: > > > > > Quoting Sean Paul (2020-01-14 17:21:43) > > > > > > From: Sean Paul > > > > > > > > > > > > This patch uses a ring_buffer to keep a "flight recorder" (name > > > > > > credit Weston) > > > > > > of DRM logs for a specified set of debug categories. The user > > > > > > writes a > > > > > > bitmask of debug categories to the "trace_mask" node and can read > > > > > > log > > > > > > messages from the "trace" node. > > > > > > > > > > > > These nodes currently exist in debugfs under the dri directory. I > > > > > > intended on exposing all of this through tracefs originally, but the > > > > > > tracefs entry points are not exposed, so there's no way to create > > > > > > tracefs files from drivers at the moment. I think it would be a > > > > > > worthwhile endeavour, but one requiring more time and conversation > > > > > > to > > > > > > ensure the drm traces fit somewhere sensible. > > > > > > > > > > Fwiw, I have a need for client orientated debug message store, with > > > > > the primary purpose of figuring out -EINVAL. We need per-client so we > > > > > can > > > > > put sensitive information about the potentially buggy client > > > > > behaviour, > > > > > and of course it needs to be accessible by the non-privileged client. > > > > > > > > > > On the execution side, it's easy to keep track of the client so we > > > > > could > > > > > trace execution flow per client, within reason. And we could do > > > > > similarly for kms clients. > > > > > > > > Could you build such a thing with drm_trace underpinning it, just put > > > > the > > > > pertinent information in the message? > > > > > > Not as is. The global has to go, and there's no use for debugfs. So we > > > are just left with a sprintf() around a ring_buffer. I am left in the > > > same position as just wanting to generalise tracek to take the ringbuffer > > > as a parameter. > > > > > > > Ah, I think I see what you're getting at now. I think it would be > > reasonable to > > split out a drm_trace_buffer from the current code for this purpose. We > > could > > have an interface like: > > > > struct drm_trace_buffer *drm_trace_buffer_init(unsigned int num_pages); > > int drm_trace_buffer_resize(struct drm_trace_buffer *buf, unsigned int > > num_pages); > > int drm_trace_buffer_printf(struct drm_trace_buffer *buf, const char > > *format, ...); > > int drm_trace_buffer_output(struct seq_file *seq); > > void drm_trace_buffer_cleanup(struct drm_trace_buffer *buf); > > > > Then to Joonas' point, we could have drm_trace_log which uses this > > interface to > > mirror the logs with a debugfs interface. > > > > Would that work for your purpose? > > The seq_file doesn't marry with the anticipated uAPI, I'll probably need > a raw file_ops (thinking along the lines of return an fd to userspace, > that is read ala /dev/kmsg). Agree, that should have been struct file_operations *drm_trace_buffer_file_ops(struct drm_trace_buffer *buf); or something like that.. > > I would be tempted to drop the drm_ and put it straight in lib/ I think if we wanted to share this more broadly, we'd probably look at adding it in kernel/trace/ and enabling subsystems to add their own traces to tracefs. Sean > -Chris -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
Quoting Sean Paul (2020-01-15 14:21:18) > On Wed, Jan 15, 2020 at 02:01:19PM +, Chris Wilson wrote: > > Quoting Sean Paul (2020-01-15 13:41:58) > > > On Wed, Jan 15, 2020 at 10:36:36AM +, Chris Wilson wrote: > > > > Quoting Sean Paul (2020-01-14 17:21:43) > > > > > From: Sean Paul > > > > > > > > > > This patch uses a ring_buffer to keep a "flight recorder" (name > > > > > credit Weston) > > > > > of DRM logs for a specified set of debug categories. The user writes a > > > > > bitmask of debug categories to the "trace_mask" node and can read log > > > > > messages from the "trace" node. > > > > > > > > > > These nodes currently exist in debugfs under the dri directory. I > > > > > intended on exposing all of this through tracefs originally, but the > > > > > tracefs entry points are not exposed, so there's no way to create > > > > > tracefs files from drivers at the moment. I think it would be a > > > > > worthwhile endeavour, but one requiring more time and conversation to > > > > > ensure the drm traces fit somewhere sensible. > > > > > > > > Fwiw, I have a need for client orientated debug message store, with > > > > the primary purpose of figuring out -EINVAL. We need per-client so we > > > > can > > > > put sensitive information about the potentially buggy client behaviour, > > > > and of course it needs to be accessible by the non-privileged client. > > > > > > > > On the execution side, it's easy to keep track of the client so we could > > > > trace execution flow per client, within reason. And we could do > > > > similarly for kms clients. > > > > > > Could you build such a thing with drm_trace underpinning it, just put the > > > pertinent information in the message? > > > > Not as is. The global has to go, and there's no use for debugfs. So we > > are just left with a sprintf() around a ring_buffer. I am left in the > > same position as just wanting to generalise tracek to take the ringbuffer > > as a parameter. > > > > Ah, I think I see what you're getting at now. I think it would be reasonable > to > split out a drm_trace_buffer from the current code for this purpose. We could > have an interface like: > > struct drm_trace_buffer *drm_trace_buffer_init(unsigned int num_pages); > int drm_trace_buffer_resize(struct drm_trace_buffer *buf, unsigned int > num_pages); > int drm_trace_buffer_printf(struct drm_trace_buffer *buf, const char *format, > ...); > int drm_trace_buffer_output(struct seq_file *seq); > void drm_trace_buffer_cleanup(struct drm_trace_buffer *buf); > > Then to Joonas' point, we could have drm_trace_log which uses this interface > to > mirror the logs with a debugfs interface. > > Would that work for your purpose? The seq_file doesn't marry with the anticipated uAPI, I'll probably need a raw file_ops (thinking along the lines of return an fd to userspace, that is read ala /dev/kmsg). I would be tempted to drop the drm_ and put it straight in lib/ -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Wed, Jan 15, 2020 at 02:01:19PM +, Chris Wilson wrote: > Quoting Sean Paul (2020-01-15 13:41:58) > > On Wed, Jan 15, 2020 at 10:36:36AM +, Chris Wilson wrote: > > > Quoting Sean Paul (2020-01-14 17:21:43) > > > > From: Sean Paul > > > > > > > > This patch uses a ring_buffer to keep a "flight recorder" (name credit > > > > Weston) > > > > of DRM logs for a specified set of debug categories. The user writes a > > > > bitmask of debug categories to the "trace_mask" node and can read log > > > > messages from the "trace" node. > > > > > > > > These nodes currently exist in debugfs under the dri directory. I > > > > intended on exposing all of this through tracefs originally, but the > > > > tracefs entry points are not exposed, so there's no way to create > > > > tracefs files from drivers at the moment. I think it would be a > > > > worthwhile endeavour, but one requiring more time and conversation to > > > > ensure the drm traces fit somewhere sensible. > > > > > > Fwiw, I have a need for client orientated debug message store, with > > > the primary purpose of figuring out -EINVAL. We need per-client so we can > > > put sensitive information about the potentially buggy client behaviour, > > > and of course it needs to be accessible by the non-privileged client. > > > > > > On the execution side, it's easy to keep track of the client so we could > > > trace execution flow per client, within reason. And we could do > > > similarly for kms clients. > > > > Could you build such a thing with drm_trace underpinning it, just put the > > pertinent information in the message? > > Not as is. The global has to go, and there's no use for debugfs. So we > are just left with a sprintf() around a ring_buffer. I am left in the > same position as just wanting to generalise tracek to take the ringbuffer > as a parameter. > Ah, I think I see what you're getting at now. I think it would be reasonable to split out a drm_trace_buffer from the current code for this purpose. We could have an interface like: struct drm_trace_buffer *drm_trace_buffer_init(unsigned int num_pages); int drm_trace_buffer_resize(struct drm_trace_buffer *buf, unsigned int num_pages); int drm_trace_buffer_printf(struct drm_trace_buffer *buf, const char *format, ...); int drm_trace_buffer_output(struct seq_file *seq); void drm_trace_buffer_cleanup(struct drm_trace_buffer *buf); Then to Joonas' point, we could have drm_trace_log which uses this interface to mirror the logs with a debugfs interface. Would that work for your purpose? > > > Just chiming to say, I don't think a duplicate of dmesg hidden inside > > > debugfs achieves much. But a generic tracek-esque ringbuf would be very > > > useful -- even if only so we can separate our GEM_TRACE from the global > > > tracek. > > > > I think that's essentially what we've got, I've just narrowly focused on > > surfacing debug logs. If drm_trace_printf were exported, replacing > > GEM_TRACE would be as simple as s/trace_printk/drm_trace_printf/. Initially > > I > > thought exporting it to drivers would be a bad idea, but I'm open to > > changing my > > mind on this as long as drivers are using it responsibly. > > I definitely can't make the mistake of flooding kms tracing with > overwhelming execution traces -- we can't go back to mixing kms traces > with execution traces. Yeah, I assumed this wouldn't be enabled during normal operation, just for debugging (as it is used now). Sean > -Chris -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
Quoting Sean Paul (2020-01-15 13:41:58) > On Wed, Jan 15, 2020 at 10:36:36AM +, Chris Wilson wrote: > > Quoting Sean Paul (2020-01-14 17:21:43) > > > From: Sean Paul > > > > > > This patch uses a ring_buffer to keep a "flight recorder" (name credit > > > Weston) > > > of DRM logs for a specified set of debug categories. The user writes a > > > bitmask of debug categories to the "trace_mask" node and can read log > > > messages from the "trace" node. > > > > > > These nodes currently exist in debugfs under the dri directory. I > > > intended on exposing all of this through tracefs originally, but the > > > tracefs entry points are not exposed, so there's no way to create > > > tracefs files from drivers at the moment. I think it would be a > > > worthwhile endeavour, but one requiring more time and conversation to > > > ensure the drm traces fit somewhere sensible. > > > > Fwiw, I have a need for client orientated debug message store, with > > the primary purpose of figuring out -EINVAL. We need per-client so we can > > put sensitive information about the potentially buggy client behaviour, > > and of course it needs to be accessible by the non-privileged client. > > > > On the execution side, it's easy to keep track of the client so we could > > trace execution flow per client, within reason. And we could do > > similarly for kms clients. > > Could you build such a thing with drm_trace underpinning it, just put the > pertinent information in the message? Not as is. The global has to go, and there's no use for debugfs. So we are just left with a sprintf() around a ring_buffer. I am left in the same position as just wanting to generalise tracek to take the ringbuffer as a parameter. > > Just chiming to say, I don't think a duplicate of dmesg hidden inside > > debugfs achieves much. But a generic tracek-esque ringbuf would be very > > useful -- even if only so we can separate our GEM_TRACE from the global > > tracek. > > I think that's essentially what we've got, I've just narrowly focused on > surfacing debug logs. If drm_trace_printf were exported, replacing > GEM_TRACE would be as simple as s/trace_printk/drm_trace_printf/. Initially I > thought exporting it to drivers would be a bad idea, but I'm open to changing > my > mind on this as long as drivers are using it responsibly. I definitely can't make the mistake of flooding kms tracing with overwhelming execution traces -- we can't go back to mixing kms traces with execution traces. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Wed, Jan 15, 2020 at 10:36:36AM +, Chris Wilson wrote: > Quoting Sean Paul (2020-01-14 17:21:43) > > From: Sean Paul > > > > This patch uses a ring_buffer to keep a "flight recorder" (name credit > > Weston) > > of DRM logs for a specified set of debug categories. The user writes a > > bitmask of debug categories to the "trace_mask" node and can read log > > messages from the "trace" node. > > > > These nodes currently exist in debugfs under the dri directory. I > > intended on exposing all of this through tracefs originally, but the > > tracefs entry points are not exposed, so there's no way to create > > tracefs files from drivers at the moment. I think it would be a > > worthwhile endeavour, but one requiring more time and conversation to > > ensure the drm traces fit somewhere sensible. > > Fwiw, I have a need for client orientated debug message store, with > the primary purpose of figuring out -EINVAL. We need per-client so we can > put sensitive information about the potentially buggy client behaviour, > and of course it needs to be accessible by the non-privileged client. > > On the execution side, it's easy to keep track of the client so we could > trace execution flow per client, within reason. And we could do > similarly for kms clients. Could you build such a thing with drm_trace underpinning it, just put the pertinent information in the message? > > Just chiming to say, I don't think a duplicate of dmesg hidden inside > debugfs achieves much. But a generic tracek-esque ringbuf would be very > useful -- even if only so we can separate our GEM_TRACE from the global > tracek. I think that's essentially what we've got, I've just narrowly focused on surfacing debug logs. If drm_trace_printf were exported, replacing GEM_TRACE would be as simple as s/trace_printk/drm_trace_printf/. Initially I thought exporting it to drivers would be a bad idea, but I'm open to changing my mind on this as long as drivers are using it responsibly. Sean > -Chris -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Wed, Jan 15, 2020 at 12:28:31PM +0200, Jani Nikula wrote:
> On Tue, 14 Jan 2020, Sean Paul wrote:
> > From: Sean Paul
> >
> > This patch uses a ring_buffer to keep a "flight recorder" (name credit
> > Weston)
> > of DRM logs for a specified set of debug categories. The user writes a
> > bitmask of debug categories to the "trace_mask" node and can read log
> > messages from the "trace" node.
> >
> > These nodes currently exist in debugfs under the dri directory. I
> > intended on exposing all of this through tracefs originally, but the
> > tracefs entry points are not exposed, so there's no way to create
> > tracefs files from drivers at the moment. I think it would be a
> > worthwhile endeavour, but one requiring more time and conversation to
> > ensure the drm traces fit somewhere sensible.
> >
> > Cc: Daniel Vetter
> > Cc: David Airlie
> > Cc: Jani Nikula
> > Cc: Joonas Lahtinen
> > Cc: Pekka Paalanen
> > Cc: Rob Clark
> > Cc: Steven Rostedt
> > Cc: Thomas Zimmermann
> > Cc: Ville Syrjälä
> > Signed-off-by: Sean Paul
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-s...@poorly.run
> > #v1
> > Link:
> > https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html
> > #v2
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-s...@poorly.run
> > #v3
> >
> > Changes in v2:
> > - Went with a completely different approach:
> > https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html
> >
> > Changes in v3:
> > - Changed commit message to be a bit less RFC-y
> > - Make class_drm_category_log an actual trace class
> >
> > Changes in v4:
> > - Instead of [ab]using trace events and the system trace buffer, use our
> > own ringbuffer
> > ---
> > ---
> > Documentation/gpu/drm-uapi.rst | 9 +
> > drivers/gpu/drm/Kconfig| 1 +
> > drivers/gpu/drm/Makefile | 2 +-
> > drivers/gpu/drm/drm_drv.c | 3 +
> > drivers/gpu/drm/drm_print.c| 80 +--
> > drivers/gpu/drm/drm_trace.c| 376 +
> > include/drm/drm_print.h| 39
> > 7 files changed, 487 insertions(+), 23 deletions(-)
> > create mode 100644 drivers/gpu/drm/drm_trace.c
> >
> > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> > index 56fec6ed1ad8..089eb6fd3e94 100644
> > --- a/Documentation/gpu/drm-uapi.rst
> > +++ b/Documentation/gpu/drm-uapi.rst
> > @@ -312,6 +312,15 @@ Debugfs Support
> > .. kernel-doc:: drivers/gpu/drm/drm_debugfs.c
> > :export:
> >
> > +DRM Tracing
> > +---
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_trace.c
> > + :doc: DRM Tracing
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_trace.c
> > + :internal:
> > +
> > Sysfs Support
> > =
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index d0aa6cff2e02..9d8077e87afe 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -14,6 +14,7 @@ menuconfig DRM
> > select I2C
> > select I2C_ALGOBIT
> > select DMA_SHARED_BUFFER
> > + select RING_BUFFER
> > select SYNC_FILE
> > help
> > Kernel-level support for the Direct Rendering Infrastructure (DRI)
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 6493088a0fdd..88b4674934e6 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -29,7 +29,7 @@ drm-$(CONFIG_DRM_PANEL) += drm_panel.o
> > drm-$(CONFIG_OF) += drm_of.o
> > drm-$(CONFIG_AGP) += drm_agpsupport.o
> > drm-$(CONFIG_PCI) += drm_pci.o
> > -drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
> > +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o drm_trace.o
> > drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> >
> > drm_vram_helper-y := drm_gem_vram_helper.o \
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 7c18a980cd4b..98260b9f8004 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -1114,6 +1114,7 @@ static const struct file_operations drm_stub_fops = {
> > static void drm_core_exit(void)
> > {
> > unregister_chrdev(DRM_MAJOR, "drm");
> > + drm_trace_cleanup();
> > debugfs_remove(drm_debugfs_root);
> > drm_sysfs_destroy();
> > idr_destroy(&drm_minors_idr);
> > @@ -1135,6 +1136,8 @@ static int __init drm_core_init(void)
> >
> > drm_debugfs_root = debugfs_create_dir("dri", NULL);
> >
> > + WARN_ON(drm_trace_init(drm_debugfs_root));
> > +
> > ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
> > if (ret < 0)
> > goto error;
> > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > index 111b932cf2a9..0ac1867937bf 100644
> > --- a/drivers/gpu/drm/drm_print.c
> > +++ b/drivers/gpu/drm/drm_print.c
> > @@ -262,21 +262,37 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > struct va_format vaf;
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Wed, Jan 15, 2020 at 12:14:50PM +0200, Pekka Paalanen wrote: > On Tue, 14 Jan 2020 12:21:43 -0500 > Sean Paul wrote: > > > From: Sean Paul > > > > This patch uses a ring_buffer to keep a "flight recorder" (name credit > > Weston) > > of DRM logs for a specified set of debug categories. The user writes a > > bitmask of debug categories to the "trace_mask" node and can read log > > messages from the "trace" node. > > > > These nodes currently exist in debugfs under the dri directory. I > > intended on exposing all of this through tracefs originally, but the > > tracefs entry points are not exposed, so there's no way to create > > tracefs files from drivers at the moment. I think it would be a > > worthwhile endeavour, but one requiring more time and conversation to > > ensure the drm traces fit somewhere sensible. > > > > Cc: Daniel Vetter > > Cc: David Airlie > > Cc: Jani Nikula > > Cc: Joonas Lahtinen > > Cc: Pekka Paalanen > > Cc: Rob Clark > > Cc: Steven Rostedt > > Cc: Thomas Zimmermann > > Cc: Ville Syrjälä > > Signed-off-by: Sean Paul > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-s...@poorly.run > > #v1 > > Link: > > https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html > > #v2 > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-s...@poorly.run > > #v3 > > > > Changes in v2: > > - Went with a completely different approach: > > https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html > > > > Changes in v3: > > - Changed commit message to be a bit less RFC-y > > - Make class_drm_category_log an actual trace class > > > > Changes in v4: > > - Instead of [ab]using trace events and the system trace buffer, use our > > own ringbuffer > > --- > > --- > > Documentation/gpu/drm-uapi.rst | 9 + > > drivers/gpu/drm/Kconfig| 1 + > > drivers/gpu/drm/Makefile | 2 +- > > drivers/gpu/drm/drm_drv.c | 3 + > > drivers/gpu/drm/drm_print.c| 80 +-- > > drivers/gpu/drm/drm_trace.c| 376 + > > include/drm/drm_print.h| 39 > > 7 files changed, 487 insertions(+), 23 deletions(-) > > create mode 100644 drivers/gpu/drm/drm_trace.c > > ... > > > +/** > > + * DOC: DRM Tracing > > + * > > + * *tl;dr* DRM tracing is a lightweight alternative to traditional DRM > > debug > > + * logging. > > + * > > + * While DRM logging is quite convenient when reproducing a specific > > issue, it > > + * doesn't help when something goes wrong unexpectedly. There are a couple > > + * reasons why one does not want to enable DRM logging at all times: > > + * > > + * 1. We don't want to overwhelm syslog with drm spam, others have to use > > it too > > + * 2. Console logging is slow > > + * > > + * DRM tracing aims to solve both these problems. > > + * > > + * To use DRM tracing, write a DRM debug category mask (this is a bitmask > > of > > + * &drm_debug_category values) to the trace_mask file: > > + * :: > > + * > > + *eg: echo 0x106 > /sys/kernel/debug/dri/trace_mask > > + * > > + * Once active, all log messages in the specified categories will be > > written to > > + * the DRM trace. Once at capacity, the trace will overwrite old messages > > with > > + * new ones. At any point, one can read the trace file to extract the > > previous N > > + * DRM messages: > > + * :: > > + * > > + *eg: cat /sys/kernel/debug/dri/trace > > + * > > + * Considerations > > + * ** > > + * The contents of the DRM Trace are **not** considered UABI. **DO NOT > > depend on > > + * the values of these traces in your userspace.** These traces are > > intended for > > + * entertainment purposes only. The contents of these logs carry no > > warranty, > > + * expressed or implied. > > + * > > + * New traces can not be added to the trace buffer while it is being read. > > If > > + * this proves to be a problem, it can be mitigated by making a copy of the > > + * buffer on start of read. Since DRM trace is not meant to be continuously > > + * read, this loss is acceptable. > > + * > > + * The timestamps on logs are CPU-local. As such, log messages from > > different > > + * CPUs may have slightly different ideas about time. > > + * > > + * Since each CPU has its own buffer, they won't all overflow at the same > > rate. > > + * This means that messages from a particularly active CPU could be dropped > > + * while an inactive CPU might have much older log messages. So don't be > > fooled > > + * if you seem to be missing log messages when you see a switch between > > CPUs in > > + * the logs. > > + * > > + * Internals > > + * * > > + * The DRM Tracing functions are intentionally unexported, they are not > > meant to > > + * be used by drivers directly. The reasons are twofold: > > + * > > + * 1. All messages going to traces should also go to the console logs. This > > + *ensures users can choose their loggi
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
Quoting Sean Paul (2020-01-14 17:21:43) > From: Sean Paul > > This patch uses a ring_buffer to keep a "flight recorder" (name credit Weston) > of DRM logs for a specified set of debug categories. The user writes a > bitmask of debug categories to the "trace_mask" node and can read log > messages from the "trace" node. > > These nodes currently exist in debugfs under the dri directory. I > intended on exposing all of this through tracefs originally, but the > tracefs entry points are not exposed, so there's no way to create > tracefs files from drivers at the moment. I think it would be a > worthwhile endeavour, but one requiring more time and conversation to > ensure the drm traces fit somewhere sensible. Fwiw, I have a need for client orientated debug message store, with the primary purpose of figuring out -EINVAL. We need per-client so we can put sensitive information about the potentially buggy client behaviour, and of course it needs to be accessible by the non-privileged client. On the execution side, it's easy to keep track of the client so we could trace execution flow per client, within reason. And we could do similarly for kms clients. Just chiming to say, I don't think a duplicate of dmesg hidden inside debugfs achieves much. But a generic tracek-esque ringbuf would be very useful -- even if only so we can separate our GEM_TRACE from the global tracek. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Tue, 14 Jan 2020, Sean Paul wrote:
> From: Sean Paul
>
> This patch uses a ring_buffer to keep a "flight recorder" (name credit Weston)
> of DRM logs for a specified set of debug categories. The user writes a
> bitmask of debug categories to the "trace_mask" node and can read log
> messages from the "trace" node.
>
> These nodes currently exist in debugfs under the dri directory. I
> intended on exposing all of this through tracefs originally, but the
> tracefs entry points are not exposed, so there's no way to create
> tracefs files from drivers at the moment. I think it would be a
> worthwhile endeavour, but one requiring more time and conversation to
> ensure the drm traces fit somewhere sensible.
>
> Cc: Daniel Vetter
> Cc: David Airlie
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Pekka Paalanen
> Cc: Rob Clark
> Cc: Steven Rostedt
> Cc: Thomas Zimmermann
> Cc: Ville Syrjälä
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-s...@poorly.run
> #v1
> Link:
> https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-s...@poorly.run
> #v3
>
> Changes in v2:
> - Went with a completely different approach:
> https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html
>
> Changes in v3:
> - Changed commit message to be a bit less RFC-y
> - Make class_drm_category_log an actual trace class
>
> Changes in v4:
> - Instead of [ab]using trace events and the system trace buffer, use our
> own ringbuffer
> ---
> ---
> Documentation/gpu/drm-uapi.rst | 9 +
> drivers/gpu/drm/Kconfig| 1 +
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_drv.c | 3 +
> drivers/gpu/drm/drm_print.c| 80 +--
> drivers/gpu/drm/drm_trace.c| 376 +
> include/drm/drm_print.h| 39
> 7 files changed, 487 insertions(+), 23 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_trace.c
>
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 56fec6ed1ad8..089eb6fd3e94 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -312,6 +312,15 @@ Debugfs Support
> .. kernel-doc:: drivers/gpu/drm/drm_debugfs.c
> :export:
>
> +DRM Tracing
> +---
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_trace.c
> + :doc: DRM Tracing
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_trace.c
> + :internal:
> +
> Sysfs Support
> =
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index d0aa6cff2e02..9d8077e87afe 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -14,6 +14,7 @@ menuconfig DRM
> select I2C
> select I2C_ALGOBIT
> select DMA_SHARED_BUFFER
> + select RING_BUFFER
> select SYNC_FILE
> help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6493088a0fdd..88b4674934e6 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -29,7 +29,7 @@ drm-$(CONFIG_DRM_PANEL) += drm_panel.o
> drm-$(CONFIG_OF) += drm_of.o
> drm-$(CONFIG_AGP) += drm_agpsupport.o
> drm-$(CONFIG_PCI) += drm_pci.o
> -drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
> +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o drm_trace.o
> drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>
> drm_vram_helper-y := drm_gem_vram_helper.o \
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7c18a980cd4b..98260b9f8004 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1114,6 +1114,7 @@ static const struct file_operations drm_stub_fops = {
> static void drm_core_exit(void)
> {
> unregister_chrdev(DRM_MAJOR, "drm");
> + drm_trace_cleanup();
> debugfs_remove(drm_debugfs_root);
> drm_sysfs_destroy();
> idr_destroy(&drm_minors_idr);
> @@ -1135,6 +1136,8 @@ static int __init drm_core_init(void)
>
> drm_debugfs_root = debugfs_create_dir("dri", NULL);
>
> + WARN_ON(drm_trace_init(drm_debugfs_root));
> +
> ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
> if (ret < 0)
> goto error;
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 111b932cf2a9..0ac1867937bf 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -262,21 +262,37 @@ void drm_dev_dbg(const struct device *dev, enum
> drm_debug_category category,
> struct va_format vaf;
> va_list args;
>
> - if (!drm_debug_enabled(category))
> - return;
> -
> - va_start(args, format);
> - vaf.fmt = format;
> - vaf.va = &args;
> -
> - if (dev)
> - dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
> -
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Tue, 14 Jan 2020 12:21:43 -0500 Sean Paul wrote: > From: Sean Paul > > This patch uses a ring_buffer to keep a "flight recorder" (name credit Weston) > of DRM logs for a specified set of debug categories. The user writes a > bitmask of debug categories to the "trace_mask" node and can read log > messages from the "trace" node. > > These nodes currently exist in debugfs under the dri directory. I > intended on exposing all of this through tracefs originally, but the > tracefs entry points are not exposed, so there's no way to create > tracefs files from drivers at the moment. I think it would be a > worthwhile endeavour, but one requiring more time and conversation to > ensure the drm traces fit somewhere sensible. > > Cc: Daniel Vetter > Cc: David Airlie > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Pekka Paalanen > Cc: Rob Clark > Cc: Steven Rostedt > Cc: Thomas Zimmermann > Cc: Ville Syrjälä > Signed-off-by: Sean Paul > Link: > https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-s...@poorly.run > #v1 > Link: > https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html #v2 > Link: > https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-s...@poorly.run > #v3 > > Changes in v2: > - Went with a completely different approach: > https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html > > Changes in v3: > - Changed commit message to be a bit less RFC-y > - Make class_drm_category_log an actual trace class > > Changes in v4: > - Instead of [ab]using trace events and the system trace buffer, use our > own ringbuffer > --- > --- > Documentation/gpu/drm-uapi.rst | 9 + > drivers/gpu/drm/Kconfig| 1 + > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/drm_drv.c | 3 + > drivers/gpu/drm/drm_print.c| 80 +-- > drivers/gpu/drm/drm_trace.c| 376 + > include/drm/drm_print.h| 39 > 7 files changed, 487 insertions(+), 23 deletions(-) > create mode 100644 drivers/gpu/drm/drm_trace.c ... > +/** > + * DOC: DRM Tracing > + * > + * *tl;dr* DRM tracing is a lightweight alternative to traditional DRM debug > + * logging. > + * > + * While DRM logging is quite convenient when reproducing a specific issue, > it > + * doesn't help when something goes wrong unexpectedly. There are a couple > + * reasons why one does not want to enable DRM logging at all times: > + * > + * 1. We don't want to overwhelm syslog with drm spam, others have to use it > too > + * 2. Console logging is slow > + * > + * DRM tracing aims to solve both these problems. > + * > + * To use DRM tracing, write a DRM debug category mask (this is a bitmask of > + * &drm_debug_category values) to the trace_mask file: > + * :: > + * > + *eg: echo 0x106 > /sys/kernel/debug/dri/trace_mask > + * > + * Once active, all log messages in the specified categories will be written > to > + * the DRM trace. Once at capacity, the trace will overwrite old messages > with > + * new ones. At any point, one can read the trace file to extract the > previous N > + * DRM messages: > + * :: > + * > + *eg: cat /sys/kernel/debug/dri/trace > + * > + * Considerations > + * ** > + * The contents of the DRM Trace are **not** considered UABI. **DO NOT > depend on > + * the values of these traces in your userspace.** These traces are intended > for > + * entertainment purposes only. The contents of these logs carry no warranty, > + * expressed or implied. > + * > + * New traces can not be added to the trace buffer while it is being read. If > + * this proves to be a problem, it can be mitigated by making a copy of the > + * buffer on start of read. Since DRM trace is not meant to be continuously > + * read, this loss is acceptable. > + * > + * The timestamps on logs are CPU-local. As such, log messages from different > + * CPUs may have slightly different ideas about time. > + * > + * Since each CPU has its own buffer, they won't all overflow at the same > rate. > + * This means that messages from a particularly active CPU could be dropped > + * while an inactive CPU might have much older log messages. So don't be > fooled > + * if you seem to be missing log messages when you see a switch between CPUs > in > + * the logs. > + * > + * Internals > + * * > + * The DRM Tracing functions are intentionally unexported, they are not > meant to > + * be used by drivers directly. The reasons are twofold: > + * > + * 1. All messages going to traces should also go to the console logs. This > + *ensures users can choose their logging medium without fear they're > losing > + *messages. > + * 2. Writing directly to the trace skips category filtering, resulting in > trace > + *spam. > + */ Hi, sounds like a good first step to me! I still have concerns about depending on debugfs in production and in desktop distributions when this feature is wanted to be on by default, but
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
Quoting Sean Paul (2020-01-14 19:21:43)
> From: Sean Paul
>
> This patch uses a ring_buffer to keep a "flight recorder" (name credit Weston)
> of DRM logs for a specified set of debug categories. The user writes a
> bitmask of debug categories to the "trace_mask" node and can read log
> messages from the "trace" node.
Would it be worthy to add more specificity to the terminology and
call this "log trace" to tell it apart from pre-existing tracing in
the context tracepoints? Especially as the proposal started as adding
tracepoints.
Further, I feel like it might make sense to add "log_trace_size"
property, or is your thinking to deliberately keep the buffer rather
small? A dump at the time of GEM error from i915 can be rather massive
(and massively useful, too).
Other than that, it looks good to me. Thanks for moving this forward :)
Regards, Joonas
> These nodes currently exist in debugfs under the dri directory. I
> intended on exposing all of this through tracefs originally, but the
> tracefs entry points are not exposed, so there's no way to create
> tracefs files from drivers at the moment. I think it would be a
> worthwhile endeavour, but one requiring more time and conversation to
> ensure the drm traces fit somewhere sensible.
>
> Cc: Daniel Vetter
> Cc: David Airlie
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Pekka Paalanen
> Cc: Rob Clark
> Cc: Steven Rostedt
> Cc: Thomas Zimmermann
> Cc: Ville Syrjälä
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-s...@poorly.run
> #v1
> Link:
> https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-s...@poorly.run
> #v3
>
> Changes in v2:
> - Went with a completely different approach:
> https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html
>
> Changes in v3:
> - Changed commit message to be a bit less RFC-y
> - Make class_drm_category_log an actual trace class
>
> Changes in v4:
> - Instead of [ab]using trace events and the system trace buffer, use our
> own ringbuffer
> ---
> ---
> Documentation/gpu/drm-uapi.rst | 9 +
> drivers/gpu/drm/Kconfig| 1 +
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_drv.c | 3 +
> drivers/gpu/drm/drm_print.c| 80 +--
> drivers/gpu/drm/drm_trace.c| 376 +
> include/drm/drm_print.h| 39
> 7 files changed, 487 insertions(+), 23 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_trace.c
>
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 56fec6ed1ad8..089eb6fd3e94 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -312,6 +312,15 @@ Debugfs Support
> .. kernel-doc:: drivers/gpu/drm/drm_debugfs.c
> :export:
>
> +DRM Tracing
> +---
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_trace.c
> + :doc: DRM Tracing
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_trace.c
> + :internal:
> +
> Sysfs Support
> =
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index d0aa6cff2e02..9d8077e87afe 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -14,6 +14,7 @@ menuconfig DRM
> select I2C
> select I2C_ALGOBIT
> select DMA_SHARED_BUFFER
> + select RING_BUFFER
> select SYNC_FILE
> help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6493088a0fdd..88b4674934e6 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -29,7 +29,7 @@ drm-$(CONFIG_DRM_PANEL) += drm_panel.o
> drm-$(CONFIG_OF) += drm_of.o
> drm-$(CONFIG_AGP) += drm_agpsupport.o
> drm-$(CONFIG_PCI) += drm_pci.o
> -drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
> +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o drm_trace.o
> drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>
> drm_vram_helper-y := drm_gem_vram_helper.o \
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7c18a980cd4b..98260b9f8004 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1114,6 +1114,7 @@ static const struct file_operations drm_stub_fops = {
> static void drm_core_exit(void)
> {
> unregister_chrdev(DRM_MAJOR, "drm");
> + drm_trace_cleanup();
> debugfs_remove(drm_debugfs_root);
> drm_sysfs_destroy();
> idr_destroy(&drm_minors_idr);
> @@ -1135,6 +1136,8 @@ static int __init drm_core_init(void)
>
> drm_debugfs_root = debugfs_create_dir("dri", NULL);
>
> + WARN_ON(drm_trace_init(drm_debugfs_root));
> +
> ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
> if (ret < 0)
> goto error;
> diff --git a/drivers/gpu/drm/d
Re: [PATCH v4] drm/trace: Buffer DRM logs in a ringbuffer accessible via debugfs
On Tue, 14 Jan 2020 12:21:43 -0500 Sean Paul wrote: > From: Sean Paul > > This patch uses a ring_buffer to keep a "flight recorder" (name credit Weston) > of DRM logs for a specified set of debug categories. The user writes a > bitmask of debug categories to the "trace_mask" node and can read log > messages from the "trace" node. > > These nodes currently exist in debugfs under the dri directory. I > intended on exposing all of this through tracefs originally, but the > tracefs entry points are not exposed, so there's no way to create > tracefs files from drivers at the moment. I think it would be a > worthwhile endeavour, but one requiring more time and conversation to > ensure the drm traces fit somewhere sensible. You may want to rebase these on my tree that renames ring_buffer to trace_buffer. For more information, read this thread: https://lore.kernel.org/r/20191213153553.GE20583@krava My tree is currently being tested and when it finishes contingent on no errors found, I'll be pushing it to linux-next. https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git/commit/?h=ftrace/core&id=13292494379f92f532de71b31a54018336adc589 -- Steve > > Cc: Daniel Vetter > Cc: David Airlie > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Pekka Paalanen > Cc: Rob Clark > Cc: Steven Rostedt > Cc: Thomas Zimmermann > Cc: Ville Syrjälä > Signed-off-by: Sean Paul > Link: > https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-s...@poorly.run > #v1 > Link: > https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html #v2 > Link: > https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-s...@poorly.run > #v3 > > Changes in v2: > - Went with a completely different approach: > https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html > > Changes in v3: > - Changed commit message to be a bit less RFC-y > - Make class_drm_category_log an actual trace class > > Changes in v4: > - Instead of [ab]using trace events and the system trace buffer, use our > own ringbuffer > --- ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
