linux-kernel - Re: [PATCH] drm/i915: annote drop_caches debugfs interface with lockdep

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170312195740.GN28427@nuc-i3427.alporthouse.com>
Date:   Sun, 12 Mar 2017 19:57:40 +0000
From:   Chris Wilson <chris@...is-wilson.co.uk>
To:     Daniel Vetter <daniel.vetter@...ll.ch>
Cc:     Intel Graphics Development <intel-gfx@...ts.freedesktop.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        Daniel Vetter <daniel.vetter@...el.com>
Subject: Re: [PATCH] drm/i915: annote drop_caches debugfs interface with
 lockdep

On Sun, Mar 12, 2017 at 08:27:16PM +0100, Daniel Vetter wrote:
> The trouble we have is that we can't really test all the shrinker
> recursion stuff exhaustively in BAT because any kind of thrashing
> stress test just takes too long.
> 
> But that leaves a really big gap open, since shrinker recursions are
> one of the most annoying bugs. Now lockdep already has support for
> checking allocation deadlocks:
> 
> - Direct reclaim paths are marked up with
>   lockdep_set_current_reclaim_state() and
>   lockdep_clear_current_reclaim_state().
> 
> - Any allocation paths are marked with lockdep_trace_alloc().
> 
> If we simply mark up our debugfs with the reclaim annotations, any
> code and locks taken in there will automatically complete the picture
> with any allocation paths we already have, as long as we have a simple
> testcase in BAT which throws out a few objects using this interface.
> Not stress test or thrashing needed at all.
> 
> v2: Need to EXPORT_SYMBOL_GPL to make it compile as a module.
> 
> Cc: Chris Wilson <chris@...is-wilson.co.uk>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: linux-kernel@...r.kernel.org
> Reviewed-by: Chris Wilson <chris@...is-wilson.co.uk> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
> 
> --
> 
> Peter/Ingo,
> 
> We want this to validate the i915 shrinker locking in our fast tests
> without thrashing badly (that takes too long, we can only thrash in
> the extended runs). Can you pls take a look and if it's ok ack for
> merging through drm-intel.git?
> 
> Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
>  kernel/locking/lockdep.c            | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 82fb005a5e22..0f1d6c4a212b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4273,11 +4273,13 @@ i915_drop_caches_set(void *data, u64 val)
>  	if (val & (DROP_RETIRE | DROP_ACTIVE))
>  		i915_gem_retire_requests(dev_priv);
>  
> +	lockdep_set_current_reclaim_state(GFP_KERNEL);
>  	if (val & DROP_BOUND)
>  		i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_BOUND);
>  
>  	if (val & DROP_UNBOUND)
>  		i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_UNBOUND);
> +	lockdep_clear_current_reclaim_state();
>  
>  	if (val & DROP_SHRINK_ALL)
>  		i915_gem_shrink_all(dev_priv);

Best to move the clear to here.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre