2021-09-02 22:10:41

by Jonathan Corbet

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

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


2021-09-02 22:10:41

by Jonathan Corbet

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

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 <[email protected]>
---
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

2021-09-02 22:10:41

by Jonathan Corbet

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

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 <[email protected]>
---
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

2021-09-03 06:35:37

by Akira Yokosawa

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

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

2021-09-03 15:12:17

by Jonathan Corbet

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

Akira Yokosawa <[email protected]> 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

2021-09-04 01:34:34

by Akira Yokosawa

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

On Fri, 03 Sep 2021 09:11:26 -0600, Jonathan Corbet wrote:
> Akira Yokosawa <[email protected]> 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
>

2021-09-04 07:51:47

by Markus Heiser

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

Am 04.09.21 um 03:23 schrieb Akira Yokosawa:
> On Fri, 03 Sep 2021 09:11:26 -0600, Jonathan Corbet wrote:
>> Akira Yokosawa <[email protected]> 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
>>

2021-09-06 14:18:33

by Jonathan Corbet

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

Markus Heiser <[email protected]> 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

2021-09-06 14:43:09

by Markus Heiser

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

Am 06.09.21 um 16:17 schrieb Jonathan Corbet:
> Markus Heiser <[email protected]> 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

2021-09-16 09:56:03

by Jan Kara

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

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 <[email protected]>

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

Reviewed-by: Jan Kara <[email protected]>

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 <[email protected]>
SUSE Labs, CR

2021-09-16 09:56:50

by Jan Kara

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

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 <[email protected]>
> ---
> 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 <[email protected]>

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 <[email protected]>
SUSE Labs, CR

2021-09-22 00:43:42

by Jonathan Corbet

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

Jan Kara <[email protected]> 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 <[email protected]>
>
> Thanks! Definitely looks more readable :). You can add:
>
> Reviewed-by: Jan Kara <[email protected]>

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

jon

2021-10-07 14:30:22

by Theodore Ts'o

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

On Tue, Sep 21, 2021 at 05:18:06PM -0600, Jonathan Corbet wrote:
> Jan Kara <[email protected]> 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 <[email protected]>
> >
> > Thanks! Definitely looks more readable :). You can add:
> >
> > Reviewed-by: Jan Kara <[email protected]>
>
> 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

2021-10-07 14:50:26

by Jonathan Corbet

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

"Theodore Ts'o" <[email protected]> writes:

> On Tue, Sep 21, 2021 at 05:18:06PM -0600, Jonathan Corbet wrote:
>> Jan Kara <[email protected]> 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 <[email protected]>
>> >
>> > Thanks! Definitely looks more readable :). You can add:
>> >
>> > Reviewed-by: Jan Kara <[email protected]>
>>
>> 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

2021-10-08 01:10:35

by Jani Nikula

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

On Thu, 02 Sep 2021, Jonathan Corbet <[email protected]> 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 <[email protected]>
> ---
> 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