On Thu, May 18, 2023 at 07:40:37AM -0700, Luis Chamberlain wrote:
> +..
> + Mapping of heading styles within this document:
> + Heading 1 uses "====" above and below
> + Heading 2 uses "===="
> + Heading 3 uses "----"
> + Heading 4 uses "````"
> + Heading 5 uses "^^^^"
> + Heading 6 uses "~~~~"
> + Heading 7 uses "...."
Everyone can distinguished those heading levels without telling them.
> +
> + Sections are manually numbered because apparently that's what everyone
> + does in the kernel.
I don't see any section numbering here (forget to add one?).
> +.. contents:: Table of Contents
> + :local:
I see the toctree before actual doc title. Maybe move down toctree below
it?
> +You can call into **iomap** for reading, ie, dealing with the filesystems's
> +``struct file_operations``:
> +
> + * ``struct file_operations.read_iter()``: note that depending on the type of read your filesystem
> + might use ``iomap_dio_rw()`` for direct IO, generic_file_read_iter() for buffered IO and
Try to be consistent on inlining code keywords (identifiers, function names, etc).
> +Testing Direct IO
> +=================
> +
> +Other than fstests you can use LTP's dio, however this tests is limited as it does not test stale
> +data.
> +
> +{{{
> +./runltp -f dio -d /mnt1/scratch/tmp/
> +}}}
Use literal code block for above shell snippet:
---- >8 ----
diff --git a/Documentation/filesystems/iomap.rst b/Documentation/filesystems/iomap.rst
index 51be574b5fe32a..6918a4cc5e3b9b 100644
--- a/Documentation/filesystems/iomap.rst
+++ b/Documentation/filesystems/iomap.rst
@@ -213,9 +213,9 @@ Testing Direct IO
Other than fstests you can use LTP's dio, however this tests is limited as it does not test stale
data.
-{{{
-./runltp -f dio -d /mnt1/scratch/tmp/
-}}}
+::
+
+ ./runltp -f dio -d /mnt1/scratch/tmp/
Known issues and future improvements
====================================
> +References
> +==========
> +
> + * `Presentation on iomap evolution`<https://docs.google.com/presentation/d/e/2PACX-1vSN4TmhiTu1c6HNv6_gJZFqbFZpbF7GkABllSwJw5iLnSYKkkO-etQJ3AySYEbgJA/pub?start=true&loop=false&delayms=3000&slide=id.g189cfd05063_0_185>`
> + * `LWN review on deprecating buffer-heads <https://lwn.net/Articles/930173/>`
I don't see clickable hyperlinks on above external references, so I have to
fix them up:
---- >8 ----
diff --git a/Documentation/filesystems/iomap.rst b/Documentation/filesystems/iomap.rst
index 75716d4e2f4537..51be574b5fe32a 100644
--- a/Documentation/filesystems/iomap.rst
+++ b/Documentation/filesystems/iomap.rst
@@ -230,5 +230,5 @@ We try to document known issues that folks should be aware of with **iomap** her
References
==========
- * `Presentation on iomap evolution`<https://docs.google.com/presentation/d/e/2PACX-1vSN4TmhiTu1c6HNv6_gJZFqbFZpbF7GkABllSwJw5iLnSYKkkO-etQJ3AySYEbgJA/pub?start=true&loop=false&delayms=3000&slide=id.g189cfd05063_0_185>`
- * `LWN review on deprecating buffer-heads <https://lwn.net/Articles/930173/>`
+ * `Presentation on iomap evolution <https://docs.google.com/presentation/d/e/2PACX-1vSN4TmhiTu1c6HNv6_gJZFqbFZpbF7GkABllSwJw5iLnSYKkkO-etQJ3AySYEbgJA/pub?start=true&loop=false&delayms=3000&slide=id.g189cfd05063_0_185>`_
+ * `LWN review on deprecating buffer-heads <https://lwn.net/Articles/930173/>`_
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index cfb724a4c65280..af5e6c0e84923f 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -25,7 +25,7 @@
*
* Block mapping provides a mapping between data cached in memory and the location on persistent
* storage where that data lives. `LWN has an great review of the old buffer-heads block-mapping and
- * why they are inefficient <https://lwn.net/Articles/930173/>`, since the inception of Linux.
+ * why they are inefficient <https://lwn.net/Articles/930173/>`_, since the inception of Linux.
* Since **buffer-heads** work on a 512-byte block based paradigm, it creates an overhead for modern
* storage media which no longer necessarily works only on 512-blocks. iomap is flexible
* providing block ranges in *bytes*. iomap, with the support of folios, provides a modern
> -/*
> - * Flags reported by the file system from iomap_begin:
> +/**
> + * DOC: Flags reported by the file system from iomap_begin
> *
> - * IOMAP_F_NEW indicates that the blocks have been newly allocated and need
> - * zeroing for areas that no data is copied to.
> + * * IOMAP_F_NEW: indicates that the blocks have been newly allocated and need
> + * zeroing for areas that no data is copied to.
> *
> - * IOMAP_F_DIRTY indicates the inode has uncommitted metadata needed to access
> - * written data and requires fdatasync to commit them to persistent storage.
> - * This needs to take into account metadata changes that *may* be made at IO
> - * completion, such as file size updates from direct IO.
> + * * IOMAP_F_DIRTY: indicates the inode has uncommitted metadata needed to access
> + * written data and requires fdatasync to commit them to persistent storage.
> + * This needs to take into account metadata changes that *may* be made at IO
> + * completion, such as file size updates from direct IO.
> *
> - * IOMAP_F_SHARED indicates that the blocks are shared, and will need to be
> - * unshared as part a write.
> + * * IOMAP_F_SHARED: indicates that the blocks are shared, and will need to be
> + * unshared as part a write.
> *
> - * IOMAP_F_MERGED indicates that the iomap contains the merge of multiple block
> - * mappings.
> + * * IOMAP_F_MERGED: indicates that the iomap contains the merge of multiple block
> + * mappings.
> *
> - * IOMAP_F_BUFFER_HEAD indicates that the file system requires the use of
> - * buffer heads for this mapping.
> + * * IOMAP_F_BUFFER_HEAD: indicates that the file system requires the use of
> + * buffer heads for this mapping.
> *
> - * IOMAP_F_XATTR indicates that the iomap is for an extended attribute extent
> - * rather than a file data extent.
> + * * IOMAP_F_XATTR: indicates that the iomap is for an extended attribute extent
> + * rather than a file data extent.
> */
Why don't use kernel-doc comments to describe flags?
> +/**
> + * struct iomap_folio_ops - buffered writes folio folio reference count helpers
> + *
> + * A filesystem can optionally set folio_ops in a &struct iomap mapping it returns to
> + * override the default get_folio and put_folio for each folio written to. This only applies
> + * to buffered writes as unbuffered writes will not typically have folios associated with them.
> + *
> + * @get_folio: iomap defaults to iomap_get_folio() (which calls __filemap_get_folio()) if the
> + * filesystem did not provide a get folio op.
> + *
> + * @put_folio: when get_folio succeeds, put_folio will always be called to do any cleanup work
> + * necessary. put_folio is responsible for unlocking and putting @folio.
> + *
> + * @iomap_valid: check that the cached iomap still maps correctly to the filesystem's internal
> + * extent map. FS internal extent maps can change while iomap is iterating a cached iomap, so
> + * this hook allows iomap to detect that the iomap needs to be refreshed during a long running
> + * write operation.
> + *
> + * The filesystem can store internal state (e.g. a sequence number) in iomap->validity_cookie
> + * when the iomap is first mapped to be able to detect changes between mapping time and whenever
> + * .iomap_valid() is called.
> + *
> + * This is called with the folio over the specified file position held locked by the iomap code.
> + * This is useful for filesystems that have dynamic mappings (e.g. anything other than zonefs).
> + * An example reason as to why this is necessary is writeback doesn't take the vfs locks.
Nice, this one has kernel-doc.
Thanks for review.
--
An old man doll... just what I always wanted! - Clara
Attachment:
signature.asc
Description: PGP signature
