Re: [PATCH] drm/i915: Disable kmem_caches when KASAN is enabled

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Wed, Mar 15, 2017 at 11:46:24AM +0200, Mika Kuoppala wrote:
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > kasan is very good at detecting use-after-free. However, our requests
>> > are allocated from a rcu-typesafe slab that is important for performance
>> > but makes kasan less effective. When enabling kasan we are intentionally
>> > looking for memory errors, disable the use of our caches to improve the
>> > likelihood of kasan spotting a bug.
>> >

I didn't notice anything except the typo in the desc. Until
someone makes a kmem_cache kasan aware, this patch would
be useful if and when someone does kasan runs with our driver.

The name could be DEBUG_KMEM_CACHE_DISABLE. It is more logical
wrt to content of the patch. However perhaps the current name
is more practical.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>


>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/Kconfig.debug      | 13 +++++++++++++
>> >  drivers/gpu/drm/i915/i915_gem.c         | 11 +++++++++--
>> >  drivers/gpu/drm/i915/i915_gem_request.c | 25 ++++++++++++++++++++-----
>> >  drivers/gpu/drm/i915/i915_vma.c         | 15 ++++++++++++---
>> >  4 files changed, 54 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
>> > index e091809a9a9e..bd8e90e4dfb9 100644
>> > --- a/drivers/gpu/drm/i915/Kconfig.debug
>> > +++ b/drivers/gpu/drm/i915/Kconfig.debug
>> > @@ -48,6 +48,19 @@ config DRM_I915_DEBUG_GEM
>> >  
>> >            If in doubt, say "N".
>> >  
>> > +config DRM_I915_DEBUG_KASAN
>> > +        bool "Insert extra checks when using KASAN"
>> > +        default n
>> > +        depends on DRM_I915_WERROR
>> > +        depends on KASAN
>> > +        help
>> > +	  Turns off use of kmem_caches to improve KASAN error detection,
>> > +          and inserts extra santiy checks.
>> 
>> s/santiy/sanity.
>> 
>> Would i915.kmem_cache=0/1 be too costly?
>
> More of the principle. Having module options means somebody will enable
> it. Hiding it away underneath a bevy of expert config options makes it
> much less likely. CI basically have to do two runs for each option -- if
> we are using kasan, we want improved coverage, but we also need to test
> that the default configuration works. It's probably best just to teach
> kmem_cache itself to behave differently if it helps. I was just thinking
> fine-grained swapping of behaviour would be benificial.
> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx