Re: linux-next: build warning after merge of the ftrace tree

Re: linux-next: build warning after merge of the ftrace tree

[Steven Rostedt]

On Mon, 16 Nov 2020 17:35:02 +1100
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> After merging the ftrace tree, today's linux-next build (htmldocs)
> produced this warning:
> 
> Documentation/trace/ftrace-uses.rst:123: WARNING: Unexpected indentation.
> 
> Introduced by commit
> 
>   a25d036d939a ("ftrace: Reverse what the RECURSION flag means in the ftrace_ops")
> 

I'm not good at rst markup. Not sure how to fix this.

Thanks,

-- Steve

Re: linux-next: build warning after merge of the ftrace tree

[Jonathan Corbet]

On Mon, 16 Nov 2020 12:43:38 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> > After merging the ftrace tree, today's linux-next build (htmldocs)
> > produced this warning:
> > 
> > Documentation/trace/ftrace-uses.rst:123: WARNING: Unexpected indentation.
> > 
> > Introduced by commit
> > 
> >   a25d036d939a ("ftrace: Reverse what the RECURSION flag means in the ftrace_ops")
> >   
> 
> I'm not good at rst markup. Not sure how to fix this.

Looking at the commit in question:

> +Protect your callback
> +=====================
> +
> +As functions can be called from anywhere, and it is possible that a function
> +called by a callback may also be traced, and call that same callback,
> +recursion protection must be used. There are two helper functions that
> +can help in this regard. If you start your code with:
> +
> +	int bit;
> +
> +	bit = ftrace_test_recursion_trylock();
> +	if (bit < 0)
> +		return;

The problem is those literal blocks.  The easiest fix will be to just use
the double-colon notation to indicate a literal block, so the paragraph
above would end with "...start your code with::".  Note that there's a few
of them to fix.

Thanks,

jon

Re: linux-next: build warning after merge of the ftrace tree

[Steven Rostedt]

On Mon, 16 Nov 2020 12:24:32 -0700
Jonathan Corbet <corbet@lwn.net> wrote:

> The problem is those literal blocks.  The easiest fix will be to just use
> the double-colon notation to indicate a literal block, so the paragraph
> above would end with "...start your code with::".  Note that there's a few
> of them to fix.

Would something like the below work? I think I fixed the other places with
issues and for consistency, replaced the ".. code-block:: c" with just "::"
usage throughout the file.

-- Steve

diff --git a/Documentation/trace/ftrace-uses.rst b/Documentation/trace/ftrace-uses.rst
index 5981d5691745..f21464538fa0 100644
--- a/Documentation/trace/ftrace-uses.rst
+++ b/Documentation/trace/ftrace-uses.rst
@@ -43,9 +43,7 @@ as well as what protections the callback will perform and not require
 ftrace to handle.
 
 There is only one field that is needed to be set when registering
-an ftrace_ops with ftrace:
-
-.. code-block:: c
+an ftrace_ops with ftrace::
 
  struct ftrace_ops ops = {
        .func			= my_callback_func,
@@ -82,9 +80,7 @@ may take some time to finish.
 The callback function
 =====================
 
-The prototype of the callback function is as follows (as of v4.14):
-
-.. code-block:: c
+The prototype of the callback function is as follows (as of v4.14)::
 
    void callback_func(unsigned long ip, unsigned long parent_ip,
                       struct ftrace_ops *op, struct pt_regs *regs);
@@ -114,7 +110,7 @@ Protect your callback
 As functions can be called from anywhere, and it is possible that a function
 called by a callback may also be traced, and call that same callback,
 recursion protection must be used. There are two helper functions that
-can help in this regard. If you start your code with:
+can help in this regard. If you start your code with::
 
 	int bit;
 
@@ -122,7 +118,7 @@ can help in this regard. If you start your code with:
 	if (bit < 0)
 		return;
 
-and end it with:
+and end it with::
 
 	ftrace_test_recursion_unlock(bit);
 
@@ -143,7 +139,7 @@ function call.
 If your callback accesses any data or critical section that requires RCU
 protection, it is best to make sure that RCU is "watching", otherwise
 that data or critical section will not be protected as expected. In this
-case add:
+case add::
 
 	if (!rcu_is_watching())
 		return;
@@ -249,9 +245,7 @@ match a specific pattern.
 
 See Filter Commands in :file:`Documentation/trace/ftrace.rst`.
 
-To just trace the schedule function:
-
-.. code-block:: c
+To just trace the schedule function::
 
    ret = ftrace_set_filter(&ops, "schedule", strlen("schedule"), 0);
 
@@ -259,17 +253,13 @@ To add more functions, call the ftrace_set_filter() more than once with the
 @reset parameter set to zero. To remove the current filter set and replace it
 with new functions defined by @buf, have @reset be non-zero.
 
-To remove all the filtered functions and trace all functions:
-
-.. code-block:: c
+To remove all the filtered functions and trace all functions::
 
    ret = ftrace_set_filter(&ops, NULL, 0, 1);
 
 
 Sometimes more than one function has the same name. To trace just a specific
-function in this case, ftrace_set_filter_ip() can be used.
-
-.. code-block:: c
+function in this case, ftrace_set_filter_ip() can be used::
 
    ret = ftrace_set_filter_ip(&ops, ip, 0, 0);
 
@@ -284,9 +274,7 @@ two lists are non-empty and contain the same functions, the callback will not
 be called by any function.
 
 An empty "notrace" list means to allow all functions defined by the filter
-to be traced.
-
-.. code-block:: c
+to be traced::
 
    int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
                           int len, int reset);
@@ -298,9 +286,7 @@ filter list, and this function does not modify the filter list.
 A non-zero @reset will clear the "notrace" list before adding functions
 that match @buf to it.
 
-Clearing the "notrace" list is the same as clearing the filter list
-
-.. code-block:: c
+Clearing the "notrace" list is the same as clearing the filter list::
 
   ret = ftrace_set_notrace(&ops, NULL, 0, 1);
 
@@ -311,9 +297,7 @@ has been registered.
 
 If a filter is in place, and the @reset is non-zero, and @buf contains a
 matching glob to functions, the switch will happen during the time of
-the ftrace_set_filter() call. At no time will all functions call the callback.
-
-.. code-block:: c
+the ftrace_set_filter() call. At no time will all functions call the callback::
 
    ftrace_set_filter(&ops, "schedule", strlen("schedule"), 1);
 
@@ -323,9 +307,7 @@ the ftrace_set_filter() call. At no time will all functions call the callback.
 
    ftrace_set_filter(&ops, "try_to_wake_up", strlen("try_to_wake_up"), 1);
 
-is not the same as:
-
-.. code-block:: c
+is not the same as::
 
    ftrace_set_filter(&ops, "schedule", strlen("schedule"), 1);
 

Re: linux-next: build warning after merge of the ftrace tree

[Jonathan Corbet]

On Mon, 16 Nov 2020 15:25:52 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 16 Nov 2020 12:24:32 -0700
> Jonathan Corbet <corbet@lwn.net> wrote:
> 
> > The problem is those literal blocks.  The easiest fix will be to just use
> > the double-colon notation to indicate a literal block, so the paragraph
> > above would end with "...start your code with::".  Note that there's a few
> > of them to fix.  
> 
> Would something like the below work? I think I fixed the other places with
> issues and for consistency, replaced the ".. code-block:: c" with just "::"
> usage throughout the file.

That will work.  It will also have the effect of removing C syntax
highlighting from the formatted output; that is fine with me, but others
are more attached to it.

jon

Re: linux-next: build warning after merge of the ftrace tree

[Steven Rostedt]

On Mon, 16 Nov 2020 13:29:29 -0700
Jonathan Corbet <corbet@lwn.net> wrote:

> > Would something like the below work? I think I fixed the other places with
> > issues and for consistency, replaced the ".. code-block:: c" with just "::"
> > usage throughout the file.  
> 
> That will work.  It will also have the effect of removing C syntax
> highlighting from the formatted output; that is fine with me, but others
> are more attached to it.

Is there a way to set a default highlighting to be C syntax?

Otherwise I'll just put it back and add the code-block statements back in,
even though there are those that will complain about the added mark up ;-)

-- Steve

[PATCH] ftrace/documentation: Fix RST C code blocks

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Some C code in the ftrace-users.rst document is missing RST C block
annotation, which has to be added.

Link: https://lore.kernel.org/r/20201116173502.392a769c@canb.auug.org.au

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/ftrace-uses.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/trace/ftrace-uses.rst b/Documentation/trace/ftrace-uses.rst
index 5981d5691745..f7d98ae5b885 100644
--- a/Documentation/trace/ftrace-uses.rst
+++ b/Documentation/trace/ftrace-uses.rst
@@ -116,6 +116,8 @@ called by a callback may also be traced, and call that same callback,
 recursion protection must be used. There are two helper functions that
 can help in this regard. If you start your code with:
 
+.. code-block:: c
+
 	int bit;
 
 	bit = ftrace_test_recursion_trylock(ip, parent_ip);
@@ -124,6 +126,8 @@ can help in this regard. If you start your code with:
 
 and end it with:
 
+.. code-block:: c
+
 	ftrace_test_recursion_unlock(bit);
 
 The code in between will be safe to use, even if it ends up calling a
@@ -145,6 +149,8 @@ protection, it is best to make sure that RCU is "watching", otherwise
 that data or critical section will not be protected as expected. In this
 case add:
 
+.. code-block:: c
+
 	if (!rcu_is_watching())
 		return;
 
-- 
2.25.4

Re: [PATCH] ftrace/documentation: Fix RST C code blocks

[Steven Rostedt]

On Wed, 18 Nov 2020 10:35:02 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> Some C code in the ftrace-users.rst document is missing RST C block
> annotation, which has to be added.
> 
> Link: https://lore.kernel.org/r/20201116173502.392a769c@canb.auug.org.au
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>

Jon,

You want to take this through your tree?

-- Steve

Re: [PATCH] ftrace/documentation: Fix RST C code blocks

[Jonathan Corbet]

On Wed, 18 Nov 2020 10:51:27 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 18 Nov 2020 10:35:02 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > 
> > Some C code in the ftrace-users.rst document is missing RST C block
> > annotation, which has to be added.
> > 
> > Link: https://lore.kernel.org/r/20201116173502.392a769c@canb.auug.org.au
> > 
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> >  
> 
> Jon,
> 
> You want to take this through your tree?

The changes that created the warning in the first place are in the ftrace
tree, right?  So it seems that the fix should be there as well; it won't
apply to the docs tree.

Thanks,

jon

Re: [PATCH] ftrace/documentation: Fix RST C code blocks

[Steven Rostedt]

On Wed, 18 Nov 2020 09:24:32 -0700
Jonathan Corbet <corbet@lwn.net> wrote:

> > Jon,
> > 
> > You want to take this through your tree?  
> 
> The changes that created the warning in the first place are in the ftrace
> tree, right?  So it seems that the fix should be there as well; it won't
> apply to the docs tree.

Bah, I forgot the report was on linux-next and not mainline.

OK, I'll take it, but can you give an ack?

-- Steve

Re: [PATCH] ftrace/documentation: Fix RST C code blocks

[Jonathan Corbet]

On Wed, 18 Nov 2020 11:26:51 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> OK, I'll take it, but can you give an ack?

Acked-by: Jonathan Corbet <corbet@lwn.net>

jon

Re: [PATCH] ftrace/documentation: Fix RST C code blocks

[Steven Rostedt]

On Wed, 18 Nov 2020 09:28:27 -0700
Jonathan Corbet <corbet@lwn.net> wrote:

> On Wed, 18 Nov 2020 11:26:51 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > OK, I'll take it, but can you give an ack?  
> 
> Acked-by: Jonathan Corbet <corbet@lwn.net>

Thanks Jon!

-- Steve