ie31200_edac: increase polling debug message level

ie31200_edac: increase polling debug message level

[Borislav Petkov]

On Thu, Jun 29, 2017 at 01:02:38PM -0400, Jason Baron wrote:
> If we are trying to be consistent across drivers, there are quite a few
> doing edac_dbg(1, ...) in mci->edac_check() as well.

Right, dumping anything in ->edac_check() does not make a whole lot of
sense to me but I'd let people decide for their own driver. So if I were
you, I'd simply remove it altogether.

So I'm open to proposals in the forms of patches. :-)

ie31200_edac: increase polling debug message level

[Borislav Petkov]

+ Tony.

On Fri, Jun 30, 2017 at 10:27:28AM -0400, Jason Baron wrote:
> I have used the mci->edac_check() in the past when I expected output and
> wasn't getting any, and wanted to make sure that the mci->edac_check() call
> was happening.
> 
> In terms of proposals, I'd throw out converting edac_dbg() to pr_debug() or
> tracepoints, where we can control each individual print dynamically, and
> thus not have to debate levels a priori. That potentially removes the
> edac_debug_level though so I'm not sure if that would break too much stuff?

Well, I don't think we need all that fancy debugging of an EDAC driver.
Once the thing has passed testing, we can disable debugging and ship it.

I mean, do you have any valid use cases where you have to enable
debugging on a production module and would need a dynamic debugging
scheme?

Don't get me wrong - I'm not necessarily against it and I've thought
about it in the past but have never got around to finding a use case
which would be compelling enough to make me rip out the archaic
debugging fun.

ie31200_edac: increase polling debug message level

[Jason Baron]

On 06/29/2017 01:14 PM, Borislav Petkov wrote:
> On Thu, Jun 29, 2017 at 01:02:38PM -0400, Jason Baron wrote:
>> If we are trying to be consistent across drivers, there are quite a few
>> doing edac_dbg(1, ...) in mci->edac_check() as well.
>
> Right, dumping anything in ->edac_check() does not make a whole lot of
> sense to me but I'd let people decide for their own driver. So if I were
> you, I'd simply remove it altogether.
>
> So I'm open to proposals in the forms of patches. :-)
>

I have used the mci->edac_check() in the past when I expected output and 
wasn't getting any, and wanted to make sure that the mci->edac_check() 
call was happening.

In terms of proposals, I'd throw out converting edac_dbg() to pr_debug() 
or tracepoints, where we can control each individual print dynamically, 
and thus not have to debate levels a priori. That potentially removes 
the edac_debug_level though so I'm not sure if that would break too much 
stuff?

Thanks,

-Jason
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

ie31200_edac: increase polling debug message level

[Jason Baron]

On 06/29/2017 07:57 AM, Borislav Petkov wrote:
> On Wed, Jun 14, 2017 at 09:58:14AM -0400, Aristeu Rozanski wrote:
>> ie31200_check() emits a debug message at the lowest debug level which
>> means you have a once per second message in the logs that isn't
>> generally interesting. Raise it to 3 to match the other places the
>> driver emits function enter/exit debug messages.
>>
>> Cc: Jason Baron <jbaron@akamai.com>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>> Signed-off-by: Aristeu Rozanski <aris@redhat.com>
>>
>> diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c
>> index 2733fb5..16fbc07 100644
>> --- a/drivers/edac/ie31200_edac.c
>> +++ b/drivers/edac/ie31200_edac.c
>> @@ -306,7 +306,7 @@ static void ie31200_check(struct mem_ctl_info *mci)
>>  {
>>  	struct ie31200_error_info info;
>>
>> -	edac_dbg(1, "MC%d\n", mci->mc_idx);
>> +	edac_dbg(3, "MC%d\n", mci->mc_idx);
>>  	ie31200_get_and_clear_error_info(mci, &info);
>>  	ie31200_process_error_info(mci, &info);
>>  }
>>
>> --
>
> Jason?
>

Hi,

Sure, Acked-by: Jason Baron <jbaron@akamai.com>

If we are trying to be consistent across drivers, there are quite a few 
doing edac_dbg(1, ...) in mci->edac_check() as well.

Thanks,

-Jason
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

ie31200_edac: increase polling debug message level

[Borislav Petkov]

On Wed, Jun 14, 2017 at 09:58:14AM -0400, Aristeu Rozanski wrote:
> ie31200_check() emits a debug message at the lowest debug level which
> means you have a once per second message in the logs that isn't
> generally interesting. Raise it to 3 to match the other places the
> driver emits function enter/exit debug messages.
> 
> Cc: Jason Baron <jbaron@akamai.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Aristeu Rozanski <aris@redhat.com>
> 
> diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c
> index 2733fb5..16fbc07 100644
> --- a/drivers/edac/ie31200_edac.c
> +++ b/drivers/edac/ie31200_edac.c
> @@ -306,7 +306,7 @@ static void ie31200_check(struct mem_ctl_info *mci)
>  {
>  	struct ie31200_error_info info;
>  
> -	edac_dbg(1, "MC%d\n", mci->mc_idx);
> +	edac_dbg(3, "MC%d\n", mci->mc_idx);
>  	ie31200_get_and_clear_error_info(mci, &info);
>  	ie31200_process_error_info(mci, &info);
>  }
> 
> --

Jason?

ie31200_edac: increase polling debug message level

[Aristeu Rozanski]

ie31200_check() emits a debug message at the lowest debug level which
means you have a once per second message in the logs that isn't
generally interesting. Raise it to 3 to match the other places the
driver emits function enter/exit debug messages.

Cc: Jason Baron <jbaron@akamai.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Aristeu Rozanski <aris@redhat.com>

diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c
index 2733fb5..16fbc07 100644
--- a/drivers/edac/ie31200_edac.c
+++ b/drivers/edac/ie31200_edac.c
@@ -306,7 +306,7 @@ static void ie31200_check(struct mem_ctl_info *mci)
 {
 	struct ie31200_error_info info;
 
-	edac_dbg(1, "MC%d\n", mci->mc_idx);
+	edac_dbg(3, "MC%d\n", mci->mc_idx);
 	ie31200_get_and_clear_error_info(mci, &info);
 	ie31200_process_error_info(mci, &info);
 }