[PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table

[PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table

[Shenghong Han]

Some syntax errors exist in "page_owner.rst". Thanks to Akira Yokosawa and
Haowen Bai for tips to help improve the documentation.

We try to fix them. Hope that the Documentation is showed as we expect.

Signed-off-by: Shenghong Han <hanshenghong2019@email.szu.edu.cn>
Fixes: edc93abbcc6d ("tools/vm/page_owner_sort.c: support sorting blocks by multiple keys")

Co-developed-by: Yixuan Cao <caoyixuan2019@email.szu.edu.cn>
Co-developed-by: Yinan Zhang <zhangyinan2019@email.szu.edu.cn>
Co-developed-by: Chongxi Zhao <zhaochongxi2019@email.szu.edu.cn>
Co-developed-by: Jiajian Ye <yejiajian2018@email.szu.edu.cn>
Co-developed-by: Yuhong Feng <yuhongf@szu.edu.cn>
---
Hello Andrew,

In Commit 57f2b54a9379 ("Documentation/vm/page_owner.rst: update the
documentation") and Commit edc93abbcc6d ("tools/vm/page_owner_sort.c:
support sorting blocks by multiple keys"), some incorrect syntax
are used, which laeds to "build warning after merge of the mm tree".
Apologize for that!

This issue is trying to fix it.

Best,

	Shenghong Han
---
---
 Documentation/vm/page_owner.rst | 67 ++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 23 deletions(-)

diff --git a/Documentation/vm/page_owner.rst b/Documentation/vm/page_owner.rst
index 25622c715..f900ab99d 100644
--- a/Documentation/vm/page_owner.rst
+++ b/Documentation/vm/page_owner.rst
@@ -171,26 +171,47 @@ Usage
 
 STANDARD FORMAT SPECIFIERS
 ==========================
-::
-
-For --sort option:
-
-	KEY		LONG		DESCRIPTION
-	p		pid		process ID
-	tg		tgid		thread group ID
-	n		name		task command name
-	st		stacktrace	stack trace of the page allocation
-	T		txt		full text of block
-	ft		free_ts		timestamp of the page when it was released
-	at		alloc_ts	timestamp of the page when it was allocated
-        ator            allocator       memory allocator for pages
-
-For --curl option:
-
-	KEY		LONG		DESCRIPTION
-	p		pid		process ID
-	tg		tgid		thread group ID
-	n		name		task command name
-	f		free		whether the page has been released or not
-	st		stacktrace	stack trace of the page allocation
-        ator            allocator       memory allocator for pages
+
+1) `Table 1`_ for the ``--sort`` option.
+
+.. table:: Table 1
+   :name: Table 1
+
+   +--------+--------------+----------------------------------------------+
+   | KEY    | LONG         | DESCRIPTION                                  |
+   +========+==============+==============================================+
+   | p      | pid          | process ID                                   |
+   +--------+--------------+----------------------------------------------+
+   | tg     | tgid         | thread group ID                              |
+   +--------+--------------+----------------------------------------------+
+   | n      | name         | task command name                            |
+   +--------+--------------+----------------------------------------------+
+   | st     | stacktrace   | stack trace of the page allocation           |
+   +--------+--------------+----------------------------------------------+
+   | T      | txt          | full text of block                           |
+   +--------+--------------+----------------------------------------------+
+   | ft     | free_ts      | timestamp of the page when it was released   |
+   +--------+--------------+----------------------------------------------+
+   | at     | alloc_ts     | timestamp of the page when it was allocated  |
+   +--------+--------------+----------------------------------------------+
+   | ator   | allocator    | memory allocator for pages                   |
+   +--------+--------------+----------------------------------------------+
+
+2) `Table 2`_ for the ``--cull`` option.
+
+.. table:: Table 2
+   :name: Table 2
+
+   +--------+--------------+----------------------------------------------+
+   | KEY    | LONG         | DESCRIPTION                                  |
+   +========+==============+==============================================+
+   | p      | pid          | process ID                                   |
+   +--------+--------------+----------------------------------------------+
+   | tg     | tgid         | thread group ID                              |
+   +--------+--------------+----------------------------------------------+
+   | n      | name         | task command name                            |
+   +--------+--------------+----------------------------------------------+
+   | st     | stacktrace   | stack trace of the page allocation           |
+   +--------+--------------+----------------------------------------------+
+   | ator   | allocator    | memory allocator for pages                   |
+   +--------+--------------+----------------------------------------------+
-- 
2.30.1

Re: [PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table

[Jonathan Corbet]

Shenghong Han <hanshenghong2019@email.szu.edu.cn> writes:

> Some syntax errors exist in "page_owner.rst". Thanks to Akira Yokosawa and
> Haowen Bai for tips to help improve the documentation.
>
> We try to fix them. Hope that the Documentation is showed as we expect.

You *have* built the docs and know that they render as expected, right? 

> Signed-off-by: Shenghong Han <hanshenghong2019@email.szu.edu.cn>
> Fixes: edc93abbcc6d ("tools/vm/page_owner_sort.c: support sorting blocks by multiple keys")
>
> Co-developed-by: Yixuan Cao <caoyixuan2019@email.szu.edu.cn>
> Co-developed-by: Yinan Zhang <zhangyinan2019@email.szu.edu.cn>
> Co-developed-by: Chongxi Zhao <zhaochongxi2019@email.szu.edu.cn>
> Co-developed-by: Jiajian Ye <yejiajian2018@email.szu.edu.cn>
> Co-developed-by: Yuhong Feng <yuhongf@szu.edu.cn>

As I mentioned the last time I saw a version of this work, if it really
took this many people to develop this one patch, then we need signoff
lines from all of them.

> ---
> Hello Andrew,
>
> In Commit 57f2b54a9379 ("Documentation/vm/page_owner.rst: update the
> documentation") and Commit edc93abbcc6d ("tools/vm/page_owner_sort.c:
> support sorting blocks by multiple keys"), some incorrect syntax
> are used, which laeds to "build warning after merge of the mm tree".
> Apologize for that!
>
> This issue is trying to fix it.
>
> Best,
>
> 	Shenghong Han
> ---
> ---
>  Documentation/vm/page_owner.rst | 67 ++++++++++++++++++++++-----------
>  1 file changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/vm/page_owner.rst b/Documentation/vm/page_owner.rst
> index 25622c715..f900ab99d 100644
> --- a/Documentation/vm/page_owner.rst
> +++ b/Documentation/vm/page_owner.rst
> @@ -171,26 +171,47 @@ Usage
>  
>  STANDARD FORMAT SPECIFIERS
>  ==========================
> -::
> -
> -For --sort option:
> -

So the simplest fix, of course, would be to just put some leading white
space before the "For" lines.  Then the literal block would be
syntactically correct.

> -	KEY		LONG		DESCRIPTION
> -	p		pid		process ID
> -	tg		tgid		thread group ID
> -	n		name		task command name
> -	st		stacktrace	stack trace of the page allocation
> -	T		txt		full text of block
> -	ft		free_ts		timestamp of the page when it was released
> -	at		alloc_ts	timestamp of the page when it was allocated
> -        ator            allocator       memory allocator for pages
> -
> -For --curl option:
> -
> -	KEY		LONG		DESCRIPTION
> -	p		pid		process ID
> -	tg		tgid		thread group ID
> -	n		name		task command name
> -	f		free		whether the page has been released or not
> -	st		stacktrace	stack trace of the page allocation
> -        ator            allocator       memory allocator for pages
> +
> +1) `Table 1`_ for the ``--sort`` option.
> +
> +.. table:: Table 1
> +   :name: Table 1

This seems like rather more markup than is really needed?  What is the
point of these tags?

> +   +--------+--------------+----------------------------------------------+
> +   | KEY    | LONG         | DESCRIPTION                                  |
> +   +========+==============+==============================================+
> +   | p      | pid          | process ID                                   |
> +   +--------+--------------+----------------------------------------------+

...and this seems over the top.  I saw a version of this that used the
simpler format:

> +	====		==========	===========
> 	KEY		LONG		DESCRIPTION
> +	====		==========	===========
> 	p		pid		process ID

That's just as easy to read and much easier to maintain, is there a
reason you moved away from it?

Thanks,

jon

Re: [PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table

[Akira Yokosawa]

Hi,

Sorry, I don't read such a broken (no line breaks at all) message.
Please check your e-mail client settings so that it can send proper
plain-text messages.

I can test the receiving side privately.
 
--
Akira
On 2022/04/30 17:13, Shenghong Han wrote:
> 0) If you have received a similar reply, please refer to the latest reply.1) Accidentally used Chinese Input Method Editor, leaving full-width symbols during the test, causing the double dashes of "--" changed
[snip]

Re: [PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table

[Shenghong Han]

0) If you have received a similar reply, please refer to the latest reply.1) Accidentally used Chinese Input Method Editor, leaving full-width symbols during the test, causing the double dashes of "--" changed into "long single dash" which should not have occurred.2) In the current document, the "STANDARD FORMAT SPECIFIERS" table does not correctly use the format of the rst document, so in the website https://www.kernel.org/doc/html/latest/vm/page_owner.html, it does not look good. Therefore, the "STANDARD FORMAT SPECIFIERS" has been adjusted using the format of the rst table. *This* is the main purpose.3) In version 1 before, the sentence look like:"Table 1 xxx(some explaination)." and "Table 2 xxx(some explaination).", in these 2 "long" sentences, using "." instead of ":".Honestly, complex sentence is unnecessary. When I tried to modify the sentence, I ignore the strict distinction between "." and ":".In short, This modification is unnecessary.I'm terribly sorry about that.4) Thanks, Shenghong Han------------------ Original ------------------From:  "Akira Yokosawa";Date:  Sat, Apr 30, 2022 02:40 PMTo:  "Shenghong Han"; "Jonathan Corbet"; Cc:  "akpm"; "baihaowen"; "seakeel"; "linux-doc"; "linux-kernel"; "caoyixuan2019"; "yejiajian2018"; "冯老师"; Subject:  Re: [PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table On 2022/04/30 3:19,Shenghong Han wrote:> Some syntax errors exist in "page_owner.rst". Thanks to Akira Yokosawa and> Haowen Bai for tips to help improve the documentation.> > We try to fix them. Hope that the Documentation is showed as we expect.> > Signed-off-by: Shenghong Han > Fixes: edc93abbcc6d ("tools/vm/page_owner_sort.c: support sorting blocks by multiple keys")> > ---> Thanks Jonathan's suggestion.> > This fix is a simpler than before.> And yes, It has built in my machine.> > Best,> > 	Shenghong Han> ---> --->  Documentation/vm/page_owner.rst | 15 ++++++++++----->  1 file changed, 10 insertions(+), 5 deletions(-)> > diff --git a/Documentation/vm/page_owner.rst b/Documentation/vm/page_owner.rst> index 25622c715..0ecb4a739 100644> --- a/Documentation/vm/page_owner.rst> +++ b/Documentation/vm/page_owner.rst> @@ -171,11 +171,12 @@ Usage>  >  STANDARD FORMAT SPECIFIERS>  ==========================> -::>  > -For --sort option:> +1) For --sort option.>  > +	====		==========	===========>  	KEY		LONG		DESCRIPTION> +	====		==========	===========>  	p		pid		process ID>  	tg		tgid		thread group ID>  	n		name		task command name> @@ -183,14 +184,18 @@ For --sort option:>  	T		txt		full text of block>  	ft		free_ts		timestamp of the page when it was released>  	at		alloc_ts	timestamp of the page when it was allocated> -        ator            allocator       memory allocator for pages> +	ator		allocator	memory allocator for pages> +	====		==========	===========>  > -For --curl option:> +2) For --curl option.>  > +	====		==========	===========>  	KEY		LONG		DESCRIPTION> +	====		==========	===========>  	p		pid		process ID>  	tg		tgid		thread group ID>  	n		name		task command name>  	f		free		whether the page has been released or not>  	st		stacktrace	stack trace of the page allocation> -        ator            allocator       memory allocator for pages> +	ator		allocator	memory allocator for pages> +	====		==========	===========So, I have actually tested this.Are you OK with the look of  1) For --sort option.and  2) For --curl option.in generated HTML or PDF docs?In literal blocks, you would see double dashes of "--".Now they are converted to so-called endash, which is a single dashslightly longer than a normal hyphen.  It looks confusing to me.To remedy this, you need inline literal markers of  1) For ``--sort`` option.and  2) For ``--curl`` option.By the way, this patch changes ":" to "." at the end of them.Are they intentional changes?  If so, why?        Thanks, Akira

Re: [PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table

[Akira Yokosawa]

On 2022/04/30 3:19,
Shenghong Han wrote:
> Some syntax errors exist in "page_owner.rst". Thanks to Akira Yokosawa and
> Haowen Bai for tips to help improve the documentation.
> 
> We try to fix them. Hope that the Documentation is showed as we expect.
> 
> Signed-off-by: Shenghong Han <hanshenghong2019@email.szu.edu.cn>
> Fixes: edc93abbcc6d ("tools/vm/page_owner_sort.c: support sorting blocks by multiple keys")
> 
> ---
> Thanks Jonathan's suggestion.
> 
> This fix is a simpler than before.
> And yes, It has built in my machine.
> 
> Best,
> 
> 	Shenghong Han
> ---
> ---
>  Documentation/vm/page_owner.rst | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/vm/page_owner.rst b/Documentation/vm/page_owner.rst
> index 25622c715..0ecb4a739 100644
> --- a/Documentation/vm/page_owner.rst
> +++ b/Documentation/vm/page_owner.rst
> @@ -171,11 +171,12 @@ Usage
>  
>  STANDARD FORMAT SPECIFIERS
>  ==========================
> -::
>  
> -For --sort option:
> +1) For --sort option.
>  
> +	====		==========	===========
>  	KEY		LONG		DESCRIPTION
> +	====		==========	===========
>  	p		pid		process ID
>  	tg		tgid		thread group ID
>  	n		name		task command name
> @@ -183,14 +184,18 @@ For --sort option:
>  	T		txt		full text of block
>  	ft		free_ts		timestamp of the page when it was released
>  	at		alloc_ts	timestamp of the page when it was allocated
> -        ator            allocator       memory allocator for pages
> +	ator		allocator	memory allocator for pages
> +	====		==========	===========
>  
> -For --curl option:
> +2) For --curl option.
>  
> +	====		==========	===========
>  	KEY		LONG		DESCRIPTION
> +	====		==========	===========
>  	p		pid		process ID
>  	tg		tgid		thread group ID
>  	n		name		task command name
>  	f		free		whether the page has been released or not
>  	st		stacktrace	stack trace of the page allocation
> -        ator            allocator       memory allocator for pages
> +	ator		allocator	memory allocator for pages
> +	====		==========	===========

So, I have actually tested this.

Are you OK with the look of

  1) For --sort option.

and

  2) For --curl option.

in generated HTML or PDF docs?

In literal blocks, you would see double dashes of "--".
Now they are converted to so-called endash, which is a single dash
slightly longer than a normal hyphen.  It looks confusing to me.

To remedy this, you need inline literal markers of

  1) For ``--sort`` option.

and

  2) For ``--curl`` option.


By the way, this patch changes ":" to "." at the end of them.
Are they intentional changes?  If so, why?

        Thanks, Akira

Re: [PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table

[Akira Yokosawa]

Hi,

On Sat, 30 Apr 2022 02:19:26 +0800,
Shenghong Han wrote:
> Subject: [PATCH] Documentation/vm/page_owner.rst: ...
This is v2 of the patch, isn't it?  If so, it should be indicated
in the prefix as "[PATCH v2]".

> Some syntax errors exist in "page_owner.rst". Thanks to Akira Yokosawa and
> Haowen Bai for tips to help improve the documentation.> 
> We try to fix them. Hope that the Documentation is showed as we expect.
In changelog, describe technical facts.  Not what you think or hope.
You need to do a respin with a better explanation at least.

Hint: A fix without using tables would be the simplest.  Please look
at the changes in the preceding section I made in commit 5603f9bdea68
("docs: vm/page_owner: use literal blocks for param description").
You are seeing the build error because a literal block ends at a next
line which starts from column 1.

I don't think Jon would dislike the simplest approach. :-)

Of course, if you guys love tables so much, I don't mind.

> 
> Signed-off-by: Shenghong Han <hanshenghong2019@email.szu.edu.cn>
> Fixes: edc93abbcc6d ("tools/vm/page_owner_sort.c: support sorting blocks by multiple keys")
SHA-1 ID of a not-yet-merged commit will likely break later in
the merge window.

Good luck!

        Thanks, Akira
> 
> ---
> Thanks Jonathan's suggestion.
> 
> This fix is a simpler than before.
> And yes, It has built in my machine.
> 
> Best,
> 
> 	Shenghong Han
> ---
> ---
>  Documentation/vm/page_owner.rst | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/vm/page_owner.rst b/Documentation/vm/page_owner.rst
> index 25622c715..0ecb4a739 100644
> --- a/Documentation/vm/page_owner.rst
> +++ b/Documentation/vm/page_owner.rst
[...]

[PATCH] Documentation/vm/page_owner.rst: Fix syntax error and Describe details using table

[Shenghong Han]

Some syntax errors exist in "page_owner.rst". Thanks to Akira Yokosawa and
Haowen Bai for tips to help improve the documentation.

We try to fix them. Hope that the Documentation is showed as we expect.

Signed-off-by: Shenghong Han <hanshenghong2019@email.szu.edu.cn>
Fixes: edc93abbcc6d ("tools/vm/page_owner_sort.c: support sorting blocks by multiple keys")

---
Thanks Jonathan's suggestion.

This fix is a simpler than before.
And yes, It has built in my machine.

Best,

	Shenghong Han
---
---
 Documentation/vm/page_owner.rst | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/Documentation/vm/page_owner.rst b/Documentation/vm/page_owner.rst
index 25622c715..0ecb4a739 100644
--- a/Documentation/vm/page_owner.rst
+++ b/Documentation/vm/page_owner.rst
@@ -171,11 +171,12 @@ Usage
 
 STANDARD FORMAT SPECIFIERS
 ==========================
-::
 
-For --sort option:
+1) For --sort option.
 
+	====		==========	===========
 	KEY		LONG		DESCRIPTION
+	====		==========	===========
 	p		pid		process ID
 	tg		tgid		thread group ID
 	n		name		task command name
@@ -183,14 +184,18 @@ For --sort option:
 	T		txt		full text of block
 	ft		free_ts		timestamp of the page when it was released
 	at		alloc_ts	timestamp of the page when it was allocated
-        ator            allocator       memory allocator for pages
+	ator		allocator	memory allocator for pages
+	====		==========	===========
 
-For --curl option:
+2) For --curl option.
 
+	====		==========	===========
 	KEY		LONG		DESCRIPTION
+	====		==========	===========
 	p		pid		process ID
 	tg		tgid		thread group ID
 	n		name		task command name
 	f		free		whether the page has been released or not
 	st		stacktrace	stack trace of the page allocation
-        ator            allocator       memory allocator for pages
+	ator		allocator	memory allocator for pages
+	====		==========	===========
-- 
2.30.1