lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 20 Mar 2023 00:57:20 +0100
From:   Andi Shyti <andi.shyti@...ux.intel.com>
To:     Andrzej Hajda <andrzej.hajda@...el.com>
Cc:     Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>, netdev@...r.kernel.org,
        intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org,
        Chris Wilson <chris@...is-wilson.co.uk>,
        Eric Dumazet <edumazet@...gle.com>,
        Chris Wilson <chris.p.wilson@...el.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [Intel-gfx] [PATCH v4 06/10] drm/i915: Separate wakeref tracking

Hi Andrzej,

On Mon, Mar 06, 2023 at 05:32:02PM +0100, Andrzej Hajda wrote:
> From: Chris Wilson <chris@...is-wilson.co.uk>
> 
> Extract the callstack tracking of intel_runtime_pm.c into its own
> utility so that that we can reuse it for other online debugging of
> scoped wakerefs.
> 
> Signed-off-by: Chris Wilson <chris@...is-wilson.co.uk>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@...el.com>
> ---
>  drivers/gpu/drm/i915/Kconfig.debug           |   9 ++
>  drivers/gpu/drm/i915/Makefile                |   4 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c      | 222 +++----------------------
>  drivers/gpu/drm/i915/intel_wakeref.h         |   2 +-
>  drivers/gpu/drm/i915/intel_wakeref_tracker.c | 234 +++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_wakeref_tracker.h |  52 ++++++
>  6 files changed, 319 insertions(+), 204 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> index 93dfb7ed970547..5fde52107e3b44 100644
> --- a/drivers/gpu/drm/i915/Kconfig.debug
> +++ b/drivers/gpu/drm/i915/Kconfig.debug
> @@ -25,6 +25,7 @@ config DRM_I915_DEBUG
>  	select PREEMPT_COUNT
>  	select I2C_CHARDEV
>  	select STACKDEPOT
> +	select STACKTRACE
>  	select DRM_DP_AUX_CHARDEV
>  	select X86_MSR # used by igt/pm_rpm
>  	select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
> @@ -37,6 +38,7 @@ config DRM_I915_DEBUG
>  	select DRM_I915_DEBUG_GEM
>  	select DRM_I915_DEBUG_GEM_ONCE
>  	select DRM_I915_DEBUG_MMIO
> +	select DRM_I915_TRACK_WAKEREF
>  	select DRM_I915_DEBUG_RUNTIME_PM
>  	select DRM_I915_SW_FENCE_DEBUG_OBJECTS
>  	select DRM_I915_SELFTEST
> @@ -227,11 +229,18 @@ config DRM_I915_DEBUG_VBLANK_EVADE
>  
>  	  If in doubt, say "N".
>  
> +config DRM_I915_TRACK_WAKEREF
> +	depends on STACKDEPOT
> +	depends on STACKTRACE
> +	bool
> +
>  config DRM_I915_DEBUG_RUNTIME_PM
>  	bool "Enable extra state checking for runtime PM"
>  	depends on DRM_I915
>  	default n
>  	select STACKDEPOT
> +	select STACKTRACE
> +	select DRM_I915_TRACK_WAKEREF
>  	help
>  	  Choose this option to turn on extra state checking for the
>  	  runtime PM functionality. This may introduce overhead during
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index b2f91a1f826858..42daff6d575a82 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -81,6 +81,10 @@ i915-$(CONFIG_DEBUG_FS) += \
>  	i915_debugfs_params.o \
>  	display/intel_display_debugfs.o \
>  	display/intel_pipe_crc.o
> +
> +i915-$(CONFIG_DRM_I915_TRACK_WAKEREF) += \
> +	intel_wakeref_tracker.o
> +

This patch, along with the previous one and two following it, is
a bit confusing. We add this file only to remove it later and
the code hops from file to file. There seem to be some extra
steps that could be avoided.

Is there room for simplification?

Thanks,
Andi

Powered by blists - more mailing lists