diff mbox

drm/i915: Don't request a bug report for unsafe module parameters

Message ID 20180506183147.2690-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson May 6, 2018, 6:31 p.m. UTC
Unsafe module parameters are just that, unsafe. If the user is foolish
enough to try them and the kernel breaks, they get to keep both pieces.
Don't ask them to file a bug report if they broke it themselves.

References: https://bugs.freedesktop.org/show_bug.cgi?id=106423
Fixes: d15d7538c6d2 ("drm/i915: Tune down init error message due to failure injection")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Jani Nikula May 7, 2018, 6:37 a.m. UTC | #1
On Sun, 06 May 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Unsafe module parameters are just that, unsafe. If the user is foolish
> enough to try them and the kernel breaks, they get to keep both pieces.
> Don't ask them to file a bug report if they broke it themselves.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106423
> Fixes: d15d7538c6d2 ("drm/i915: Tune down init error message due to failure injection")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Acked-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/i915_drv.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9b782045ae17..9c449b8d8eab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -101,7 +101,13 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
>  		   __builtin_return_address(0), &vaf);
>  
>  	if (is_error && !shown_bug_once) {
> -		dev_notice(kdev, "%s", FDO_BUG_MSG);
> +		/*
> +		 * Ask the user to file a bug report for the error, except
> +		 * if they may have caused the bug by fiddling with unsafe
> +		 * module parameters.
> +		 */
> +		if (!test_taint(TAINT_USER))
> +			dev_notice(kdev, "%s", FDO_BUG_MSG);
>  		shown_bug_once = true;
>  	}
Joonas Lahtinen May 7, 2018, 8:03 a.m. UTC | #2
Quoting Chris Wilson (2018-05-06 21:31:47)
> Unsafe module parameters are just that, unsafe. If the user is foolish
> enough to try them and the kernel breaks, they get to keep both pieces.
> Don't ask them to file a bug report if they broke it themselves.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106423
> Fixes: d15d7538c6d2 ("drm/i915: Tune down init error message due to failure injection")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9b782045ae17..9c449b8d8eab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -101,7 +101,13 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
>                    __builtin_return_address(0), &vaf);
>  
>         if (is_error && !shown_bug_once) {
> -               dev_notice(kdev, "%s", FDO_BUG_MSG);
> +               /*
> +                * Ask the user to file a bug report for the error, except
> +                * if they may have caused the bug by fiddling with unsafe
> +                * module parameters.
> +                */
> +               if (!test_taint(TAINT_USER))
> +                       dev_notice(kdev, "%s", FDO_BUG_MSG);

We cold even have FDO_TAINT_MSG.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

>                 shown_bug_once = true;
>         }
>  
> -- 
> 2.17.0
>
Rodrigo Vivi May 7, 2018, 7:45 p.m. UTC | #3
On Sun, May 06, 2018 at 07:31:47PM +0100, Chris Wilson wrote:
> Unsafe module parameters are just that, unsafe. If the user is foolish
> enough to try them and the kernel breaks, they get to keep both pieces.
> Don't ask them to file a bug report if they broke it themselves.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106423
> Fixes: d15d7538c6d2 ("drm/i915: Tune down init error message due to failure injection")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

I believe there are cases where having the reports would be extremely useful,
like on PSR case.
But of course prioritized as "Lowest".

Better than not having anything and suddenly, when we switch
things on, we start having so many reports at once.

So, probably a different message would be worth trying?

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9b782045ae17..9c449b8d8eab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -101,7 +101,13 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
>  		   __builtin_return_address(0), &vaf);
>  
>  	if (is_error && !shown_bug_once) {
> -		dev_notice(kdev, "%s", FDO_BUG_MSG);
> +		/*
> +		 * Ask the user to file a bug report for the error, except
> +		 * if they may have caused the bug by fiddling with unsafe
> +		 * module parameters.
> +		 */
> +		if (!test_taint(TAINT_USER))
> +			dev_notice(kdev, "%s", FDO_BUG_MSG);
>  		shown_bug_once = true;
>  	}
>  
> -- 
> 2.17.0
>
Jani Nikula May 8, 2018, 10:03 a.m. UTC | #4
On Mon, 07 May 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Sun, May 06, 2018 at 07:31:47PM +0100, Chris Wilson wrote:
>> Unsafe module parameters are just that, unsafe. If the user is foolish
>> enough to try them and the kernel breaks, they get to keep both pieces.
>> Don't ask them to file a bug report if they broke it themselves.
>> 
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=106423
>> Fixes: d15d7538c6d2 ("drm/i915: Tune down init error message due to failure injection")
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> I believe there are cases where having the reports would be extremely useful,
> like on PSR case.
> But of course prioritized as "Lowest".
>
> Better than not having anything and suddenly, when we switch
> things on, we start having so many reports at once.
>
> So, probably a different message would be worth trying?

AFAICT this only gets used via i915_load_error() when driver load fails.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 9b782045ae17..9c449b8d8eab 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -101,7 +101,13 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
>>  		   __builtin_return_address(0), &vaf);
>>  
>>  	if (is_error && !shown_bug_once) {
>> -		dev_notice(kdev, "%s", FDO_BUG_MSG);
>> +		/*
>> +		 * Ask the user to file a bug report for the error, except
>> +		 * if they may have caused the bug by fiddling with unsafe
>> +		 * module parameters.
>> +		 */
>> +		if (!test_taint(TAINT_USER))
>> +			dev_notice(kdev, "%s", FDO_BUG_MSG);
>>  		shown_bug_once = true;
>>  	}
>>  
>> -- 
>> 2.17.0
>>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9b782045ae17..9c449b8d8eab 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -101,7 +101,13 @@  __i915_printk(struct drm_i915_private *dev_priv, const char *level,
 		   __builtin_return_address(0), &vaf);
 
 	if (is_error && !shown_bug_once) {
-		dev_notice(kdev, "%s", FDO_BUG_MSG);
+		/*
+		 * Ask the user to file a bug report for the error, except
+		 * if they may have caused the bug by fiddling with unsafe
+		 * module parameters.
+		 */
+		if (!test_taint(TAINT_USER))
+			dev_notice(kdev, "%s", FDO_BUG_MSG);
 		shown_bug_once = true;
 	}