[PATCH 0/2] ext4: docs: de-uglify Documentation/ext4/orphan.rst

[PATCH 0/2] ext4: docs: de-uglify Documentation/ext4/orphan.rst

[Jonathan Corbet]

I happened to see the new orphan.rst file go by and noticed it contains a
couple of pathologies that make the file far harder to read than it really
needs to be.  These patches fix things up and, hopefully, serve as a
template for equivalent changes elsewhere in this directory.

Jonathan Corbet (2):
  ext4: docs: switch away from list-table
  ext4: docs: Take out unneeded escaping

 Documentation/filesystems/ext4/orphan.rst | 44 +++++++++--------------
 1 file changed, 17 insertions(+), 27 deletions(-)

-- 
2.31.1

[PATCH 1/2] ext4: docs: switch away from list-table

[Jonathan Corbet]

Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
a new document on orphan files, which is great.  But the use of
"list-table" results in documents that are absolutely unreadable in their
plain-text form.  Switch this file to the regular RST table format instead;
the rendered (HTML) output is identical.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 Documentation/filesystems/ext4/orphan.rst | 32 ++++++++---------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/Documentation/filesystems/ext4/orphan.rst b/Documentation/filesystems/ext4/orphan.rst
index bb19ecd1b626..d096fe0ba19e 100644
--- a/Documentation/filesystems/ext4/orphan.rst
+++ b/Documentation/filesystems/ext4/orphan.rst
@@ -21,27 +21,17 @@ in heavy creation of orphan inodes. When orphan file feature
 (referenced from the superblock through s\_orphan_file_inum) with several
 blocks. Each of these blocks has a structure:
 
-.. list-table::
-   :widths: 8 8 24 40
-   :header-rows: 1
-
-   * - Offset
-     - Type
-     - Name
-     - Description
-   * - 0x0
-     - Array of \_\_le32 entries
-     - Orphan inode entries
-     - Each \_\_le32 entry is either empty (0) or it contains inode number of
-       an orphan inode.
-   * - blocksize - 8
-     - \_\_le32
-     - ob\_magic
-     - Magic value stored in orphan block tail (0x0b10ca04)
-   * - blocksize - 4
-     - \_\_le32
-     - ob\_checksum
-     - Checksum of the orphan block.
+============= ================ =============== ===============================
+Offset        Type             Name            Description
+============= ================ =============== ===============================
+0x0           Array of         Orphan inode    Each \_\_le32 entry is either
+              \_\_le32 entries entries         empty (0) or it contains
+	                                       inode number of an orphan
+					       inode.
+blocksize-8   \_\_le32         ob\_magic       Magic value stored in orphan
+                                               block tail (0x0b10ca04)
+blocksize-4   \_\_le32         ob\_checksum    Checksum of the orphan block.
+============= ================ =============== ===============================
 
 When a filesystem with orphan file feature is writeably mounted, we set
 RO\_COMPAT\_ORPHAN\_PRESENT feature in the superblock to indicate there may
-- 
2.31.1

[PATCH 2/2] ext4: docs: Take out unneeded escaping

[Jonathan Corbet]

The new file Documentation/orphan/ext4.rst escapes underscores ("\_")
throughout.  However, RST doesn't actually require that, so the escaping
only succeeds in making the document less readable.  Remove the unneeded
escapes.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 Documentation/filesystems/ext4/orphan.rst | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/filesystems/ext4/orphan.rst b/Documentation/filesystems/ext4/orphan.rst
index d096fe0ba19e..03cca178864b 100644
--- a/Documentation/filesystems/ext4/orphan.rst
+++ b/Documentation/filesystems/ext4/orphan.rst
@@ -12,31 +12,31 @@ track the inode as orphan so that in case of crash extra blocks allocated to
 the file get truncated.
 
 Traditionally ext4 tracks orphan inodes in a form of single linked list where
-superblock contains the inode number of the last orphan inode (s\_last\_orphan
+superblock contains the inode number of the last orphan inode (s_last_orphan
 field) and then each inode contains inode number of the previously orphaned
-inode (we overload i\_dtime inode field for this). However this filesystem
+inode (we overload i_dtime inode field for this). However this filesystem
 global single linked list is a scalability bottleneck for workloads that result
 in heavy creation of orphan inodes. When orphan file feature
-(COMPAT\_ORPHAN\_FILE) is enabled, the filesystem has a special inode
-(referenced from the superblock through s\_orphan_file_inum) with several
+(COMPAT_ORPHAN_FILE) is enabled, the filesystem has a special inode
+(referenced from the superblock through s_orphan_file_inum) with several
 blocks. Each of these blocks has a structure:
 
 ============= ================ =============== ===============================
 Offset        Type             Name            Description
 ============= ================ =============== ===============================
-0x0           Array of         Orphan inode    Each \_\_le32 entry is either
-              \_\_le32 entries entries         empty (0) or it contains
+0x0           Array of         Orphan inode    Each __le32 entry is either
+              __le32 entries   entries         empty (0) or it contains
 	                                       inode number of an orphan
 					       inode.
-blocksize-8   \_\_le32         ob\_magic       Magic value stored in orphan
+blocksize-8   __le32           ob_magic        Magic value stored in orphan
                                                block tail (0x0b10ca04)
-blocksize-4   \_\_le32         ob\_checksum    Checksum of the orphan block.
+blocksize-4   __le32           ob_checksum     Checksum of the orphan block.
 ============= ================ =============== ===============================
 
 When a filesystem with orphan file feature is writeably mounted, we set
-RO\_COMPAT\_ORPHAN\_PRESENT feature in the superblock to indicate there may
+RO_COMPAT_ORPHAN_PRESENT feature in the superblock to indicate there may
 be valid orphan entries. In case we see this feature when mounting the
 filesystem, we read the whole orphan file and process all orphan inodes found
 there as usual. When cleanly unmounting the filesystem we remove the
-RO\_COMPAT\_ORPHAN\_PRESENT feature to avoid unnecessary scanning of the orphan
+RO_COMPAT_ORPHAN_PRESENT feature to avoid unnecessary scanning of the orphan
 file and also make the filesystem fully compatible with older kernels.
-- 
2.31.1

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Akira Yokosawa]

Hi Jon,

On Thu,  2 Sep 2021 16:08:53 -0600, Jonathan Corbet wrote:

> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
> a new document on orphan files, which is great.  But the use of
> "list-table" results in documents that are absolutely unreadable in their
> plain-text form.  Switch this file to the regular RST table format instead;
> the rendered (HTML) output is identical.

In the "list tables" section of doc-guide/sphinx.rst, the first paragraph
starts with the sentence:

    We recommend the use of list table formats.

Yes, the disadvantage of list tables is mentioned later in the paragraph:

    Compared to the ASCII-art they might not be as comfortable for readers
    of the text files.

, but I still see list-table is meant as the preferred format.

If you prefer the ASCII-art form for simple tables, you might need to
de-emphasize the above mentioned recommendation as well.

        Thanks, Akira

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Jonathan Corbet]

Akira Yokosawa <akiyks@gmail.com> writes:

[Adding Mauro]

> On Thu,  2 Sep 2021 16:08:53 -0600, Jonathan Corbet wrote:
>
>> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
>> a new document on orphan files, which is great.  But the use of
>> "list-table" results in documents that are absolutely unreadable in their
>> plain-text form.  Switch this file to the regular RST table format instead;
>> the rendered (HTML) output is identical.
>
> In the "list tables" section of doc-guide/sphinx.rst, the first paragraph
> starts with the sentence:
>
>     We recommend the use of list table formats.
>
> Yes, the disadvantage of list tables is mentioned later in the paragraph:
>
>     Compared to the ASCII-art they might not be as comfortable for readers
>     of the text files.
>
> , but I still see list-table is meant as the preferred format.

Interesting...that is not at all my memory of the discussions we had at
that time.  There was a lot of pushback against anything that makes the
RST files less readable - still is, if certain people join the
conversation.  Tables were one of the early flash points.  

Mauro, you added that text; do you remember things differently?  Do you
feel we should retain that recommendation?

Thanks,

jon

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Akira Yokosawa]

On Fri, 03 Sep 2021 09:11:26 -0600, Jonathan Corbet wrote:
> Akira Yokosawa <akiyks@gmail.com> writes:
> 
> [Adding Mauro]
> 
>> On Thu,  2 Sep 2021 16:08:53 -0600, Jonathan Corbet wrote:
>>
>>> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
>>> a new document on orphan files, which is great.  But the use of
>>> "list-table" results in documents that are absolutely unreadable in their
>>> plain-text form.  Switch this file to the regular RST table format instead;
>>> the rendered (HTML) output is identical.
>>
>> In the "list tables" section of doc-guide/sphinx.rst, the first paragraph
>> starts with the sentence:
>>
>>     We recommend the use of list table formats.
>>
>> Yes, the disadvantage of list tables is mentioned later in the paragraph:
>>
>>     Compared to the ASCII-art they might not be as comfortable for readers
>>     of the text files.
>>
>> , but I still see list-table is meant as the preferred format.
> 
> Interesting...that is not at all my memory of the discussions we had at
> that time.  There was a lot of pushback against anything that makes the
> RST files less readable - still is, if certain people join the
> conversation.  Tables were one of the early flash points.  
> 
> Mauro, you added that text; do you remember things differently?  Do you
> feel we should retain that recommendation?

No, the text was first added by Markus Heiser [added to CC] in commit
0249a7644857 ("doc-rst: flat-table directive - initial implementation")
and have not updated ever since.

He might remember the circumstances, but 2016 was a long time ago,
I guess.

Or did the discussions take place after the list table support had been
added?

        Thanks, Akira (a newcomer to kerneldoc)

> 
> Thanks,
> 
> jon
> 

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Markus Heiser]

Am 04.09.21 um 03:23 schrieb Akira Yokosawa:
> On Fri, 03 Sep 2021 09:11:26 -0600, Jonathan Corbet wrote:
>> Akira Yokosawa <akiyks@gmail.com> writes:
>>
>> [Adding Mauro]
>>
>>> On Thu,  2 Sep 2021 16:08:53 -0600, Jonathan Corbet wrote:
>>>
>>>> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
>>>> a new document on orphan files, which is great.  But the use of
>>>> "list-table" results in documents that are absolutely unreadable in their
>>>> plain-text form.  Switch this file to the regular RST table format instead;
>>>> the rendered (HTML) output is identical.
>>>
>>> In the "list tables" section of doc-guide/sphinx.rst, the first paragraph
>>> starts with the sentence:
>>>
>>>      We recommend the use of list table formats.
>>>
>>> Yes, the disadvantage of list tables is mentioned later in the paragraph:
>>>
>>>      Compared to the ASCII-art they might not be as comfortable for readers
>>>      of the text files.
>>>
>>> , but I still see list-table is meant as the preferred format.
>>
>> Interesting...that is not at all my memory of the discussions we had at
>> that time.  There was a lot of pushback against anything that makes the
>> RST files less readable - still is, if certain people join the
>> conversation.  Tables were one of the early flash points.
>>
>> Mauro, you added that text; do you remember things differently?  Do you
>> feel we should retain that recommendation?
> 
> No, the text was first added by Markus Heiser [added to CC] in commit
> 0249a7644857 ("doc-rst: flat-table directive - initial implementation")
> and have not updated ever since.
> 
> He might remember the circumstances, but 2016 was a long time ago,
> I guess.

We prefer list tables ...

"""Their advantage is that they are easy to create or modify and that the
diff of a modification is much more meaningful, because it is limited to
the modified content."""

By example: We have some very large tables with tons of rows and cols.
If you need to extend one column just by one character you have to edit
the whole table and the diff is not readable.

It is not limited to big tables, e.g. if you patch a simple typo,
you might need touch content not related to your fix.

At the end it is a trade of, what weights more, readability of the
plain text or readability of the patch / most often I would vote
for the latter.

  -- Markus --


> 
> Or did the discussions take place after the list table support had been
> added?
> 
>          Thanks, Akira (a newcomer to kerneldoc)
> 
>>
>> Thanks,
>>
>> jon
>>

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Jonathan Corbet]

Markus Heiser <markus.heiser@darmarit.de> writes:

> We prefer list tables ...
>
> """Their advantage is that they are easy to create or modify and that the
> diff of a modification is much more meaningful, because it is limited to
> the modified content."""
>
> By example: We have some very large tables with tons of rows and cols.
> If you need to extend one column just by one character you have to edit
> the whole table and the diff is not readable.
>
> It is not limited to big tables, e.g. if you patch a simple typo,
> you might need touch content not related to your fix.
>
> At the end it is a trade of, what weights more, readability of the
> plain text or readability of the patch / most often I would vote
> for the latter.

If the documentation is of any use of all there will be a lot more
people reading it than will be reading patches making tweaks to it.
Optimizing for patch readability seems like the wrong focus to me.

The ext4 folks can decide what they like best in this specific case.
But I think that the advice in favor of list tables is wrong in the
general case; they are completely unreadable in their source form, and
that goes against one of the key reasons we adopted RST in the first
place.

Somebody will surely try to add a list table to the wrong document
someday and I'll get to live through another one of those nifty
explosions - and I'll have neither reasons nor motivation to defend that
policy.

Thanks,

jon

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Markus Heiser]

Am 06.09.21 um 16:17 schrieb Jonathan Corbet:
> Markus Heiser <markus.heiser@darmarit.de> writes:
> 
>> We prefer list tables ...
>>
>> """Their advantage is that they are easy to create or modify and that the
>> diff of a modification is much more meaningful, because it is limited to
>> the modified content."""
>>
>> By example: We have some very large tables with tons of rows and cols.
>> If you need to extend one column just by one character you have to edit
>> the whole table and the diff is not readable.
>>
>> It is not limited to big tables, e.g. if you patch a simple typo,
>> you might need touch content not related to your fix.
>>
>> At the end it is a trade of, what weights more, readability of the
>> plain text or readability of the patch / most often I would vote
>> for the latter.
> 
> If the documentation is of any use of all there will be a lot more
> people reading it than will be reading patches making tweaks to it.
> Optimizing for patch readability seems like the wrong focus to me.
> 
> The ext4 folks can decide what they like best in this specific case.
> But I think that the advice in favor of list tables is wrong in the
> general case; they are completely unreadable in their source form, and
> that goes against one of the key reasons we adopted RST in the first
> place.
> 
> Somebody will surely try to add a list table to the wrong document
> someday and I'll get to live through another one of those nifty
> explosions - and I'll have neither reasons nor motivation to defend that
> policy.

I do not see a problem changing the policy to use pre-formated tables.

@jon do you like to fix the "list tables" section of doc-guide/sphinx.rst

Thanks,

Markus

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Jan Kara]

On Thu 02-09-21 16:08:53, Jonathan Corbet wrote:
> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
> a new document on orphan files, which is great.  But the use of
> "list-table" results in documents that are absolutely unreadable in their
> plain-text form.  Switch this file to the regular RST table format instead;
> the rendered (HTML) output is identical.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>

Thanks! Definitely looks more readable :). You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  Documentation/filesystems/ext4/orphan.rst | 32 ++++++++---------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/filesystems/ext4/orphan.rst b/Documentation/filesystems/ext4/orphan.rst
> index bb19ecd1b626..d096fe0ba19e 100644
> --- a/Documentation/filesystems/ext4/orphan.rst
> +++ b/Documentation/filesystems/ext4/orphan.rst
> @@ -21,27 +21,17 @@ in heavy creation of orphan inodes. When orphan file feature
>  (referenced from the superblock through s\_orphan_file_inum) with several
>  blocks. Each of these blocks has a structure:
>  
> -.. list-table::
> -   :widths: 8 8 24 40
> -   :header-rows: 1
> -
> -   * - Offset
> -     - Type
> -     - Name
> -     - Description
> -   * - 0x0
> -     - Array of \_\_le32 entries
> -     - Orphan inode entries
> -     - Each \_\_le32 entry is either empty (0) or it contains inode number of
> -       an orphan inode.
> -   * - blocksize - 8
> -     - \_\_le32
> -     - ob\_magic
> -     - Magic value stored in orphan block tail (0x0b10ca04)
> -   * - blocksize - 4
> -     - \_\_le32
> -     - ob\_checksum
> -     - Checksum of the orphan block.
> +============= ================ =============== ===============================
> +Offset        Type             Name            Description
> +============= ================ =============== ===============================
> +0x0           Array of         Orphan inode    Each \_\_le32 entry is either
> +              \_\_le32 entries entries         empty (0) or it contains
> +	                                       inode number of an orphan
> +					       inode.
> +blocksize-8   \_\_le32         ob\_magic       Magic value stored in orphan
> +                                               block tail (0x0b10ca04)
> +blocksize-4   \_\_le32         ob\_checksum    Checksum of the orphan block.
> +============= ================ =============== ===============================
>  
>  When a filesystem with orphan file feature is writeably mounted, we set
>  RO\_COMPAT\_ORPHAN\_PRESENT feature in the superblock to indicate there may
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/2] ext4: docs: Take out unneeded escaping

[Jan Kara]

On Thu 02-09-21 16:08:54, Jonathan Corbet wrote:
> The new file Documentation/orphan/ext4.rst escapes underscores ("\_")
> throughout.  However, RST doesn't actually require that, so the escaping
> only succeeds in making the document less readable.  Remove the unneeded
> escapes.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  Documentation/filesystems/ext4/orphan.rst | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Hum, probably I saw underscores escaped somewhere and didn't check whether
it is necessary. Thanks for fixing this. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> diff --git a/Documentation/filesystems/ext4/orphan.rst b/Documentation/filesystems/ext4/orphan.rst
> index d096fe0ba19e..03cca178864b 100644
> --- a/Documentation/filesystems/ext4/orphan.rst
> +++ b/Documentation/filesystems/ext4/orphan.rst
> @@ -12,31 +12,31 @@ track the inode as orphan so that in case of crash extra blocks allocated to
>  the file get truncated.
>  
>  Traditionally ext4 tracks orphan inodes in a form of single linked list where
> -superblock contains the inode number of the last orphan inode (s\_last\_orphan
> +superblock contains the inode number of the last orphan inode (s_last_orphan
>  field) and then each inode contains inode number of the previously orphaned
> -inode (we overload i\_dtime inode field for this). However this filesystem
> +inode (we overload i_dtime inode field for this). However this filesystem
>  global single linked list is a scalability bottleneck for workloads that result
>  in heavy creation of orphan inodes. When orphan file feature
> -(COMPAT\_ORPHAN\_FILE) is enabled, the filesystem has a special inode
> -(referenced from the superblock through s\_orphan_file_inum) with several
> +(COMPAT_ORPHAN_FILE) is enabled, the filesystem has a special inode
> +(referenced from the superblock through s_orphan_file_inum) with several
>  blocks. Each of these blocks has a structure:
>  
>  ============= ================ =============== ===============================
>  Offset        Type             Name            Description
>  ============= ================ =============== ===============================
> -0x0           Array of         Orphan inode    Each \_\_le32 entry is either
> -              \_\_le32 entries entries         empty (0) or it contains
> +0x0           Array of         Orphan inode    Each __le32 entry is either
> +              __le32 entries   entries         empty (0) or it contains
>  	                                       inode number of an orphan
>  					       inode.
> -blocksize-8   \_\_le32         ob\_magic       Magic value stored in orphan
> +blocksize-8   __le32           ob_magic        Magic value stored in orphan
>                                                 block tail (0x0b10ca04)
> -blocksize-4   \_\_le32         ob\_checksum    Checksum of the orphan block.
> +blocksize-4   __le32           ob_checksum     Checksum of the orphan block.
>  ============= ================ =============== ===============================
>  
>  When a filesystem with orphan file feature is writeably mounted, we set
> -RO\_COMPAT\_ORPHAN\_PRESENT feature in the superblock to indicate there may
> +RO_COMPAT_ORPHAN_PRESENT feature in the superblock to indicate there may
>  be valid orphan entries. In case we see this feature when mounting the
>  filesystem, we read the whole orphan file and process all orphan inodes found
>  there as usual. When cleanly unmounting the filesystem we remove the
> -RO\_COMPAT\_ORPHAN\_PRESENT feature to avoid unnecessary scanning of the orphan
> +RO_COMPAT_ORPHAN_PRESENT feature to avoid unnecessary scanning of the orphan
>  file and also make the filesystem fully compatible with older kernels.
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Jonathan Corbet]

Jan Kara <jack@suse.cz> writes:

> On Thu 02-09-21 16:08:53, Jonathan Corbet wrote:
>> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
>> a new document on orphan files, which is great.  But the use of
>> "list-table" results in documents that are absolutely unreadable in their
>> plain-text form.  Switch this file to the regular RST table format instead;
>> the rendered (HTML) output is identical.
>> 
>> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
>
> Thanks! Definitely looks more readable :). You can add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks for having a look!  I'll ahead and apply these, then.

jon

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Theodore Ts'o]

On Tue, Sep 21, 2021 at 05:18:06PM -0600, Jonathan Corbet wrote:
> Jan Kara <jack@suse.cz> writes:
> 
> > On Thu 02-09-21 16:08:53, Jonathan Corbet wrote:
> >> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
> >> a new document on orphan files, which is great.  But the use of
> >> "list-table" results in documents that are absolutely unreadable in their
> >> plain-text form.  Switch this file to the regular RST table format instead;
> >> the rendered (HTML) output is identical.
> >> 
> >> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> >
> > Thanks! Definitely looks more readable :). You can add:
> >
> > Reviewed-by: Jan Kara <jack@suse.cz>
> 
> Thanks for having a look!  I'll ahead and apply these, then.

Hey Jon,

I don't see these patches in linux-next.  I'm guessing because you
were busy with some silly thing like LPC.  :-)

Do you want to take them, or I can take them through the ext4 tree.

	       	       	    	       	   - Ted

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Jonathan Corbet]

"Theodore Ts'o" <tytso@mit.edu> writes:

> On Tue, Sep 21, 2021 at 05:18:06PM -0600, Jonathan Corbet wrote:
>> Jan Kara <jack@suse.cz> writes:
>> 
>> > On Thu 02-09-21 16:08:53, Jonathan Corbet wrote:
>> >> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
>> >> a new document on orphan files, which is great.  But the use of
>> >> "list-table" results in documents that are absolutely unreadable in their
>> >> plain-text form.  Switch this file to the regular RST table format instead;
>> >> the rendered (HTML) output is identical.
>> >> 
>> >> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
>> >
>> > Thanks! Definitely looks more readable :). You can add:
>> >
>> > Reviewed-by: Jan Kara <jack@suse.cz>
>> 
>> Thanks for having a look!  I'll ahead and apply these, then.
>
> Hey Jon,
>
> I don't see these patches in linux-next.  I'm guessing because you
> were busy with some silly thing like LPC.  :-)
>
> Do you want to take them, or I can take them through the ext4 tree.

Ah...I learned something today.  If you try to apply your own patches
with "git am -s" (via b4 to lazily grab Jan's Reviewed-by tags), it
fails, complaining about duplicate signoff lines.  I'd failed to notice
that before.  I've *really* applied them this time (and tweaked my
scripts :).

Thanks,

jon

b4 and git am -s errors (was Re: [PATCH 1/2] ext4: docs: switch away from list-table)

[Konstantin Ryabitsev]

On Thu, Oct 07, 2021 at 08:48:41AM -0600, Jonathan Corbet wrote:
> Ah...I learned something today.  If you try to apply your own patches
> with "git am -s" (via b4 to lazily grab Jan's Reviewed-by tags), it
> fails, complaining about duplicate signoff lines.  I'd failed to notice
> that before.  I've *really* applied them this time (and tweaked my
> scripts :).

Butting myself into the conversation in a totally non-creepy all-seeing way.

Was that when using "git am" directly, or via "b4 shazam"?

-K

Re: b4 and git am -s errors (was Re: [PATCH 1/2] ext4: docs: switch away from list-table)

[Jonathan Corbet]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> On Thu, Oct 07, 2021 at 08:48:41AM -0600, Jonathan Corbet wrote:
>> Ah...I learned something today.  If you try to apply your own patches
>> with "git am -s" (via b4 to lazily grab Jan's Reviewed-by tags), it
>> fails, complaining about duplicate signoff lines.  I'd failed to notice
>> that before.  I've *really* applied them this time (and tweaked my
>> scripts :).
>
> Butting myself into the conversation in a totally non-creepy all-seeing way.

You can run but you can't hide :)

> Was that when using "git am" directly, or via "b4 shazam"?

My script is still "b4 am | git am", haven't tried shazam yet - it's
definitely on the radar, though.

Thanks,

jon

Re: b4 and git am -s errors (was Re: [PATCH 1/2] ext4: docs: switch away from list-table)

[Konstantin Ryabitsev]

On Thu, Oct 07, 2021 at 12:06:00PM -0600, Jonathan Corbet wrote:
> My script is still "b4 am | git am", haven't tried shazam yet - it's
> definitely on the radar, though.

Okay, now I'm curious which version of git this is, since I can't seem to
replicate this problem -- I just get:

- multiple identical Signed-off-by lines if my S-o-b is not the last
- nothing added to trailers if my S-o-b is already the last trailer

-K

Re: b4 and git am -s errors (was Re: [PATCH 1/2] ext4: docs: switch away from list-table)

[Jonathan Corbet]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> On Thu, Oct 07, 2021 at 12:06:00PM -0600, Jonathan Corbet wrote:
>> My script is still "b4 am | git am", haven't tried shazam yet - it's
>> definitely on the radar, though.
>
> Okay, now I'm curious which version of git this is, since I can't seem to
> replicate this problem -- I just get:
>
> - multiple identical Signed-off-by lines if my S-o-b is not the last
> - nothing added to trailers if my S-o-b is already the last trailer

2.31.1, as packaged by Fedora.  b4 0.8.0 from PyPI, for what that's worth.

Specifically, I do:

  b4 am -o ~/tmp -n am -c -t 20210902220854.198850-1-corbet@lwn.net
  git am -a ~/tmp/am.mbx

...and that prints "Duplicate Signed-off-by lines." and leaves the repo
in the "in the middle of an am session" state.

jon

Re: [PATCH 1/2] ext4: docs: switch away from list-table

[Jani Nikula]

On Thu, 02 Sep 2021, Jonathan Corbet <corbet@lwn.net> wrote:
> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added
> a new document on orphan files, which is great.  But the use of
> "list-table" results in documents that are absolutely unreadable in their
> plain-text form.  Switch this file to the regular RST table format instead;
> the rendered (HTML) output is identical.
>
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  Documentation/filesystems/ext4/orphan.rst | 32 ++++++++---------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/filesystems/ext4/orphan.rst b/Documentation/filesystems/ext4/orphan.rst
> index bb19ecd1b626..d096fe0ba19e 100644
> --- a/Documentation/filesystems/ext4/orphan.rst
> +++ b/Documentation/filesystems/ext4/orphan.rst
> @@ -21,27 +21,17 @@ in heavy creation of orphan inodes. When orphan file feature
>  (referenced from the superblock through s\_orphan_file_inum) with several
>  blocks. Each of these blocks has a structure:
>  
> -.. list-table::
> -   :widths: 8 8 24 40
> -   :header-rows: 1
> -
> -   * - Offset
> -     - Type
> -     - Name
> -     - Description
> -   * - 0x0
> -     - Array of \_\_le32 entries
> -     - Orphan inode entries
> -     - Each \_\_le32 entry is either empty (0) or it contains inode number of
> -       an orphan inode.
> -   * - blocksize - 8
> -     - \_\_le32
> -     - ob\_magic
> -     - Magic value stored in orphan block tail (0x0b10ca04)
> -   * - blocksize - 4
> -     - \_\_le32
> -     - ob\_checksum
> -     - Checksum of the orphan block.
> +============= ================ =============== ===============================
> +Offset        Type             Name            Description
> +============= ================ =============== ===============================
> +0x0           Array of         Orphan inode    Each \_\_le32 entry is either
> +              \_\_le32 entries entries         empty (0) or it contains
> +	                                       inode number of an orphan
> +					       inode.
> +blocksize-8   \_\_le32         ob\_magic       Magic value stored in orphan
> +                                               block tail (0x0b10ca04)
> +blocksize-4   \_\_le32         ob\_checksum    Checksum of the orphan block.
> +============= ================ =============== ===============================
>  
>  When a filesystem with orphan file feature is writeably mounted, we set
>  RO\_COMPAT\_ORPHAN\_PRESENT feature in the superblock to indicate there may

As a third alternative, the csv-table directive [1] is sometimes a good
choice. Picking | as the delim makes it look more like a table in the
source, and you don't have to worry about aligning everything (the
spaces before and after the delim are ignored by default). But it does
require some boilerplate and you can't wrap the lines.

The same table as an example:

.. csv-table:: Block Structure
   :delim: |
   :header-rows: 1
   :widths: auto

   Offset        | Type                    | Name                 | Description
   0x0           | Array of __le32 entries | Orphan inode entries | Each __le32 entry is either empty (0) or it contains inode number of an orphan inode.
   blocksize-8   | __le32                  | ob_magic             | Magic value stored in orphan block tail (0x0b10ca04)
   blocksize-4   | __le32                  | ob_checksum          | Checksum of the orphan block.

Obviously not the best choice for this particular table, but just so you
are aware of an alternative.


BR,
Jani.


[1] https://docutils.sourceforge.io/docs/ref/rst/directives.html#csv-table

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: b4 and git am -s errors (was Re: [PATCH 1/2] ext4: docs: switch away from list-table)

[Konstantin Ryabitsev]

On Thu, Oct 07, 2021 at 01:11:11PM -0600, Jonathan Corbet wrote:
>   b4 am -o ~/tmp -n am -c -t 20210902220854.198850-1-corbet@lwn.net
>   git am -a ~/tmp/am.mbx
> 
> ...and that prints "Duplicate Signed-off-by lines." and leaves the repo
> in the "in the middle of an am session" state.

Ah, looks like you have .git/hooks/commit-msg -- the default one will check
for duplicate S-o-b lines.

Not much I can do here, I'm afraid, but thanks for helping me trace what's
going on.

-K