From: Bagas Sanjaya <bagasdotme@gmail.com>
To: Russell Harmon <eatnumber1@gmail.com>,
mpatocka@redhat.com, snitzer@redhat.com, dm-devel@redhat.com
Cc: Linux Documentation <linux-doc@vger.kernel.org>
Subject: Re: [dm-devel] [PATCH] Improve the dm-integrity documentation.
Date: Sat, 3 Jun 2023 19:45:56 +0700 [thread overview]
Message-ID: <ZHs2BHGBrJ-YGd-v@debian.me> (raw)
In-Reply-To: <20230530002032.15227-1-eatnumber1@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5664 bytes --]
On Mon, May 29, 2023 at 05:20:32PM -0700, Russell Harmon wrote:
> Documents the meaning of the "buffer", adds documentation of the current
> defaults, and provides an example of how the tunables relate to each
> other.
What about splitting this patch into three-patch series, where each
patch do one improv?
Anyway, I'm reviewing the wording here.
> diff --git a/Documentation/admin-guide/device-mapper/dm-integrity.rst b/Documentation/admin-guide/device-mapper/dm-integrity.rst
> index 8db172efa272..634a780d7c51 100644
> --- a/Documentation/admin-guide/device-mapper/dm-integrity.rst
> +++ b/Documentation/admin-guide/device-mapper/dm-integrity.rst
> @@ -25,7 +25,7 @@ mode it calculates and verifies the integrity tag internally. In this
> mode, the dm-integrity target can be used to detect silent data
> corruption on the disk or in the I/O path.
>
> -There's an alternate mode of operation where dm-integrity uses bitmap
> +There's an alternate mode of operation where dm-integrity uses a bitmap
OK.
> instead of a journal. If a bit in the bitmap is 1, the corresponding
> region's data and integrity tags are not synchronized - if the machine
> crashes, the unsynchronized regions will be recalculated. The bitmap mode
> @@ -38,6 +38,15 @@ the device. But it will only format the device if the superblock contains
> zeroes. If the superblock is neither valid nor zeroed, the dm-integrity
> target can't be loaded.
>
> +Accesses to the on-disk metadata area containing checksums (aka tags) is
"Accesses to ... are ..."
> +buffered using dm-bufio. When an access to any given metadata area
> +occurs, each unique metadata area gets its own buffer(s). The buffer size
> +is capped at the size of the metadata area, but may be smaller, thereby
> +requiring multiple buffers to represent the full metadata area. A smaller
> +buffer size will produce a smaller resulting read/write operation to the
> +metadata area for small reads/writes. A full write to the data covered by
> +a single buffer does *not* skip the read of the metadata.
What about "The metadata is still read even in a full write to the data
covered by a single buffer."?
> +
> To use the target for the first time:
>
> 1. overwrite the superblock with zeroes
> @@ -93,7 +102,7 @@ journal_sectors:number
> device. If the device is already formatted, the value from the
> superblock is used.
>
> -interleave_sectors:number
> +interleave_sectors:number (default 32768)
> The number of interleaved sectors. This values is rounded down to
> a power of two. If the device is already formatted, the value from
> the superblock is used.
> @@ -102,20 +111,16 @@ meta_device:device
> Don't interleave the data and metadata on the device. Use a
> separate device for metadata.
>
> -buffer_sectors:number
> - The number of sectors in one buffer. The value is rounded down to
> - a power of two.
> +buffer_sectors:number (default 128)
> + The number of sectors in one metadata buffer. The value is rounded
> + down to a power of two.
>
> - The tag area is accessed using buffers, the buffer size is
> - configurable. The large buffer size means that the I/O size will
> - be larger, but there could be less I/Os issued.
> -
> -journal_watermark:number
> +journal_watermark:number (default 50)
> The journal watermark in percents. When the size of the journal
> exceeds this watermark, the thread that flushes the journal will
> be started.
>
> -commit_time:number
> +commit_time:number (default 10000)
> Commit time in milliseconds. When this time passes, the journal is
> written. The journal is also written immediately if the FLUSH
> request is received.
> @@ -163,11 +168,10 @@ journal_mac:algorithm(:key) (the key is optional)
> the journal. Thus, modified sector number would be detected at
> this stage.
>
> -block_size:number
> - The size of a data block in bytes. The larger the block size the
> +block_size:number (default 512)
> + The size of a data block in bytes. The larger the block size the
> less overhead there is for per-block integrity metadata.
> - Supported values are 512, 1024, 2048 and 4096 bytes. If not
> - specified the default block size is 512 bytes.
> + Supported values are 512, 1024, 2048 and 4096 bytes.
>
> sectors_per_bit:number
> In the bitmap mode, this parameter specifies the number of
> @@ -209,6 +213,12 @@ table and swap the tables with suspend and resume). The other arguments
> should not be changed when reloading the target because the layout of disk
> data depend on them and the reloaded target would be non-functional.
>
> +For example, on a device using the default interleave_sectors of 32768, a
> +block_size of 512, and an internal_hash of crc32c with a tag size of 4
> +bytes, it will take 128 KiB of tags to track a full data area, requiring
> +256 sectors of metadata per data area. With the default buffer_sectors of
> +128, that means there will be 2 buffers per metadata area, or 2 buffers
> +per 16 MiB of data.
OK.
>
> Status line:
>
> @@ -285,8 +295,8 @@ The layout of the formatted block device:
> * one or more runs of interleaved tags and data.
> Each run contains:
>
> - * tag area - it contains integrity tags. There is one tag for each
> - sector in the data area
> + * tag area - it contains integrity tags. There is one tag for each sector in
> + the data area. The size of this area is always 4KiB or greater.
Corresponding sector in the data area?
Thanks.
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next parent reply other threads:[~2023-06-03 12:46 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20230530002032.15227-1-eatnumber1@gmail.com>
2023-06-03 12:45 ` Bagas Sanjaya [this message]
2023-06-03 20:15 ` [PATCH] Improve the dm-integrity documentation Russell Harmon
2023-06-03 20:15 ` [PATCH 1/4] Fix minor grammatical error in dm-integrity.rst Russell Harmon
2023-06-03 20:15 ` [PATCH 2/4] Documents the meaning of "buffer" in dm-integrity Russell Harmon
2023-06-03 20:15 ` [PATCH 3/4] Document dm-integrity default values Russell Harmon
2023-06-03 20:15 ` [PATCH 4/4] Document an example of how the tunables relate in dm-integrity Russell Harmon
2023-06-04 14:07 ` [PATCH] Improve the dm-integrity documentation Bagas Sanjaya
2023-06-04 18:25 ` Russell Harmon
2023-06-04 19:06 ` [PATCH v3 0/4] " Russell Harmon
2023-06-04 19:06 ` [PATCH v3 1/4] Fix minor grammatical error in dm-integrity.rst Russell Harmon
2023-06-05 3:03 ` Bagas Sanjaya
2023-06-05 5:00 ` Russell Harmon
2023-06-04 19:06 ` [PATCH v3 2/4] Documents the meaning of "buffer" in dm-integrity Russell Harmon
2023-06-05 3:05 ` Bagas Sanjaya
2023-06-05 3:07 ` Bagas Sanjaya
2023-06-05 5:01 ` Russell Harmon
2023-06-04 19:06 ` [PATCH v3 3/4] Document dm-integrity default values Russell Harmon
2023-06-05 3:16 ` Bagas Sanjaya
2023-06-05 5:05 ` Russell Harmon
2023-06-05 13:23 ` Jonathan Corbet
2023-06-06 2:16 ` Bagas Sanjaya
2023-06-04 19:06 ` [PATCH v3 4/4] Document an example of how the tunables relate in dm-integrity Russell Harmon
2023-06-05 3:17 ` Bagas Sanjaya
2023-06-05 5:05 ` Russell Harmon
2023-06-05 5:08 ` [PATCH v4 0/4] Improve the dm-integrity documentation Russell Harmon
2023-06-05 5:08 ` [PATCH v4 1/4] Documentation: dm-integrity: Fix minor grammatical error Russell Harmon
2023-06-06 2:17 ` Bagas Sanjaya
2023-06-05 5:08 ` [PATCH v4 2/4] Documentation: dm-integrity: Document the meaning of "buffer" Russell Harmon
2023-06-06 2:18 ` Bagas Sanjaya
2023-06-05 5:08 ` [PATCH v4 3/4] Documentation: dm-integrity: Document default values Russell Harmon
2023-06-05 5:08 ` [PATCH v4 4/4] Documentation: dm-integrity: Document an example of how the tunables relate Russell Harmon
2023-06-17 19:37 ` Russell Harmon
[not found] ` <CA+zrezTKon+02mfMRsW34Tkovqn3FsSD2_9tk-+a4icjt9PsDg@mail.gmail.com>
2023-06-03 12:50 ` [dm-devel] [PATCH] Improve the dm-integrity documentation Bagas Sanjaya
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZHs2BHGBrJ-YGd-v@debian.me \
--to=bagasdotme@gmail.com \
--cc=dm-devel@redhat.com \
--cc=eatnumber1@gmail.com \
--cc=linux-doc@vger.kernel.org \
--cc=mpatocka@redhat.com \
--cc=snitzer@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).