[PATCH] sched/debug:fix stale comments of sched_debug

[PATCH] sched/debug:fix stale comments of sched_debug

[Hailong Liu]

From: Hailong Liu <liu.hailong6@zte.com.cn>

Now /proc/sched_debug has been moved to debugfs, so make the comments
consistent with it.

Signed-off-by: Hailong Liu <liu.hailong6@zte.com.cn>
---
 kernel/sched/debug.c | 2 +-
 lib/Kconfig.debug    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 9c882f20803e..4cbd6d602829 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -9,7 +9,7 @@
 #include "sched.h"
 
 /*
- * This allows printing both to /proc/sched_debug and
+ * This allows printing both to debugfs/sched/debug and
  * to the console
  */
 #define SEQ_printf(m, x...)			\
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 678c13967580..bb4bc59b0920 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1166,7 +1166,7 @@ config SCHED_DEBUG
 	depends on DEBUG_KERNEL && PROC_FS
 	default y
 	help
-	  If you say Y here, the /proc/sched_debug file will be provided
+	  If you say Y here, the debugfs/sched/debug file will be provided
 	  that can help debug the scheduler. The runtime overhead of this
 	  option is minimal.
 
-- 
2.17.1

Re: [PATCH] sched/debug:fix stale comments of sched_debug

[Andy Shevchenko]

On Sun, May 30, 2021 at 10:51:45AM +0800, Hailong Liu wrote:
> From: Hailong Liu <liu.hailong6@zte.com.cn>
> 
> Now /proc/sched_debug has been moved to debugfs, so make the comments
> consistent with it.

Have you checked if the very same Kconfig option is used for something
slightly different as well?

> Signed-off-by: Hailong Liu <liu.hailong6@zte.com.cn>

Missed SoB tag of the submitter.

...

> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1166,7 +1166,7 @@ config SCHED_DEBUG
>  	depends on DEBUG_KERNEL && PROC_FS

Are the dependencies correct?

>  	default y
>  	help
> -	  If you say Y here, the /proc/sched_debug file will be provided
> +	  If you say Y here, the debugfs/sched/debug file will be provided
>  	  that can help debug the scheduler. The runtime overhead of this
>  	  option is minimal.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] sched/debug:fix stale comments of sched_debug

[Hailong Liu]

On 5/31/21 3:48 PM, Andy Shevchenko wrote:
> On Sun, May 30, 2021 at 10:51:45AM +0800, Hailong Liu wrote:
>> From: Hailong Liu <liu.hailong6@zte.com.cn>
>>
>> Now /proc/sched_debug has been moved to debugfs, so make the comments
>> consistent with it.
> 
> Have you checked if the very same Kconfig option is used for something
> slightly different as well?

Do you mean that I need to check whether SCHED_DEBUG is also related to
other features? 

>> Signed-off-by: Hailong Liu <liu.hailong6@zte.com.cn>
> 
> Missed SoB tag of the submitter.

In fact the exiting SoB tag of this patch is the actual submitter. 

I'm sorry that the inconsistency between the submission email and the SoB
email address caused you to feel confused. 

For some personal reasons, I had to use an email at home and mark another
email address as the submitter. 

> 
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1166,7 +1166,7 @@ config SCHED_DEBUG
>>  	depends on DEBUG_KERNEL && PROC_FS
> 
> Are the dependencies correct?

Based on your suggestion, I checked and it turned out that PROC_FS is not
the correct dependency, but DEBUG_FS. 

Shoud I change it to DEBUG_FS? 

>>  	default y
>>  	help
>> -	  If you say Y here, the /proc/sched_debug file will be provided
>> +	  If you say Y here, the debugfs/sched/debug file will be provided
>>  	  that can help debug the scheduler. The runtime overhead of this
>>  	  option is minimal.
> 

Thanks,

Hailong

Re: [PATCH] sched/debug:fix stale comments of sched_debug

[Peter Zijlstra]

On Mon, May 31, 2021 at 10:09:36PM +0800, Hailong Liu wrote:
> Based on your suggestion, I checked and it turned out that PROC_FS is not
> the correct dependency, but DEBUG_FS. 

You should check again...

Re: [PATCH] sched/debug:fix stale comments of sched_debug

[Andy Shevchenko]

On Mon, May 31, 2021 at 10:09:36PM +0800, Hailong Liu wrote:
> On 5/31/21 3:48 PM, Andy Shevchenko wrote:
> > On Sun, May 30, 2021 at 10:51:45AM +0800, Hailong Liu wrote:
> >> From: Hailong Liu <liu.hailong6@zte.com.cn>
> >>
> >> Now /proc/sched_debug has been moved to debugfs, so make the comments
> >> consistent with it.
> > 
> > Have you checked if the very same Kconfig option is used for something
> > slightly different as well?
> 
> Do you mean that I need to check whether SCHED_DEBUG is also related to
> other features? 

Yes.

> >> Signed-off-by: Hailong Liu <liu.hailong6@zte.com.cn>
> > 
> > Missed SoB tag of the submitter.
> 
> In fact the exiting SoB tag of this patch is the actual submitter. 
> 
> I'm sorry that the inconsistency between the submission email and the SoB
> email address caused you to feel confused. 
> 
> For some personal reasons, I had to use an email at home and mark another
> email address as the submitter. 

AFAIU the documentation you have to have two SoBs in this case.

> >> --- a/lib/Kconfig.debug
> >> +++ b/lib/Kconfig.debug
> >> @@ -1166,7 +1166,7 @@ config SCHED_DEBUG
> >>  	depends on DEBUG_KERNEL && PROC_FS
> > 
> > Are the dependencies correct?
> 
> Based on your suggestion, I checked and it turned out that PROC_FS is not
> the correct dependency, but DEBUG_FS. 

I guess you missed something.

> Shoud I change it to DEBUG_FS?

Please, double check.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] sched/debug:fix stale comments of sched_debug

[Peter Zijlstra]

On Mon, May 31, 2021 at 07:05:22PM +0300, Andy Shevchenko wrote:
> > >> --- a/lib/Kconfig.debug
> > >> +++ b/lib/Kconfig.debug
> > >> @@ -1166,7 +1166,7 @@ config SCHED_DEBUG
> > >>  	depends on DEBUG_KERNEL && PROC_FS
> > > 
> > > Are the dependencies correct?
> > 
> > Based on your suggestion, I checked and it turned out that PROC_FS is not
> > the correct dependency, but DEBUG_FS. 
> 
> I guess you missed something.
> 
> > Shoud I change it to DEBUG_FS?
> 
> Please, double check.

Thing is that proc_sched_show_task() still wants PROC_FS, i've not yet
found a suitable debugfs based location for that.

And DEBUG_FS doesn't need a dependency; it has complete DEBUG_FS=n
wrappers and will build fine. Arguably selecting SCHED_DEBUG without
DEBUG_FS is daft, but it should work.

Re: [PATCH] sched/debug:fix stale comments of sched_debug

[Andy Shevchenko]

On Tue, Jun 01, 2021 at 09:56:51AM +0200, Peter Zijlstra wrote:
> On Mon, May 31, 2021 at 07:05:22PM +0300, Andy Shevchenko wrote:
> > > >> --- a/lib/Kconfig.debug
> > > >> +++ b/lib/Kconfig.debug
> > > >> @@ -1166,7 +1166,7 @@ config SCHED_DEBUG
> > > >>  	depends on DEBUG_KERNEL && PROC_FS
> > > > 
> > > > Are the dependencies correct?
> > > 
> > > Based on your suggestion, I checked and it turned out that PROC_FS is not
> > > the correct dependency, but DEBUG_FS. 
> > 
> > I guess you missed something.
> > 
> > > Shoud I change it to DEBUG_FS?
> > 
> > Please, double check.
> 
> Thing is that proc_sched_show_task() still wants PROC_FS, i've not yet
> found a suitable debugfs based location for that.
> 
> And DEBUG_FS doesn't need a dependency; it has complete DEBUG_FS=n
> wrappers and will build fine. Arguably selecting SCHED_DEBUG without
> DEBUG_FS is daft, but it should work.

Yep, my point here is that we have several files (in procfs and debugfs) which
are enabled by the same option. Changing help text as it's done in the patch
seems a half baked solution, i.e. it needs more, like describing what the files
are visible in different configurations.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] sched/debug:fix stale comments of sched_debug

[Hailong Liu]

On 6/1/21 8:58 PM, Andy Shevchenko wrote:
> On Tue, Jun 01, 2021 at 09:56:51AM +0200, Peter Zijlstra wrote:
>> On Mon, May 31, 2021 at 07:05:22PM +0300, Andy Shevchenko wrote:
>>>>>> --- a/lib/Kconfig.debug
>>>>>> +++ b/lib/Kconfig.debug
>>>>>> @@ -1166,7 +1166,7 @@ config SCHED_DEBUG
>>>>>>  	depends on DEBUG_KERNEL && PROC_FS
>>>>>
>>>>> Are the dependencies correct?
>>>>
>>>> Based on your suggestion, I checked and it turned out that PROC_FS is not
>>>> the correct dependency, but DEBUG_FS. 
>>>
>>> I guess you missed something.
>>>
>>>> Shoud I change it to DEBUG_FS?
>>>
>>> Please, double check.
>>
>> Thing is that proc_sched_show_task() still wants PROC_FS, i've not yet
>> found a suitable debugfs based location for that.
>>
>> And DEBUG_FS doesn't need a dependency; it has complete DEBUG_FS=n
>> wrappers and will build fine. Arguably selecting SCHED_DEBUG without
>> DEBUG_FS is daft, but it should work.
> 
> Yep, my point here is that we have several files (in procfs and debugfs) which
> are enabled by the same option. Changing help text as it's done in the patch
> seems a half baked solution, i.e. it needs more, like describing what the files
> are visible in different configurations.
> 

Big thanks to Andy and Peter for the views and suggestions.

Personally, neither DEBUG_FS nor RPOC_FS are the building dependencies of
SCHED_DEBUG. However, The visibility of the files(in procfs and debugfs) depends
on both.

So, can I make a compromise and add both PROC_FS and DEBUG_FS as dependencies in
Kconfig; at the same time, add more descriptions of the two situations in the
help text?