On Thu, May 18, 2023 at 08:01:05AM -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 "...."
> +
> + Sections are manually numbered because apparently that's what everyone
Why are you picking different defaults then the rest of the kernel
documentation?
> +
> +A modern block abstraction
> +==========================
> +
> +**iomap** allows filesystems to query storage media for data using *byte
> +ranges*. Since block mapping are provided for a *byte ranges* for cache data in
> +memory, in the page cache, naturally this implies operations on block ranges
> +will also deal with *multipage* operations in the page cache. **Folios** are
> +used to help provide *multipage* operations in memory for the *byte ranges*
> +being worked on.
As mentioned you list time this information was circulated this is not
true. iomap itself has nothing to with blocks, and even less so with
the page cache per se. It just iterates over ranges of file data and
applies work to it.
> +iomap IO interfaces
> +===================
> +
> +You call **iomap** depending on the type of filesystem operation you are working
> +on. We detail some of these interactions below.
Who is you?
> +
> +iomap for bufferred IO writes
> +-----------------------------
> +
> +You call **iomap** for buffered IO with:
> +
> + * ``iomap_file_buffered_write()`` - for buffered writes
> + * ``iomap_page_mkwrite()`` - when dealing callbacks for
> + ``struct vm_operations_struct``
> +
> + * ``struct vm_operations_struct.page_mkwrite()``
> + * ``struct vm_operations_struct.fault()``
> + * ``struct vm_operations_struct.huge_fault()``
> + * ``struct vm_operations_struct`.pfn_mkwrite()``
> +
> +You *may* use buffered writes to also deal with ``fallocate()``:
> +
> + * ``iomap_zero_range()`` on fallocate for zeroing
> + * ``iomap_truncate_page()`` on fallocate for truncation
> +
> +Typically you'd also happen to use these on paths when updating an inode's size.
I'm not really sure what this is trying to explain. It basically looks
like filler text generated by machine learning algorithms..
The same is true for a large part of this document.
> +A filesystem also needs to call **iomap** when assisting the VFS manipulating a
> +file into the page cache.
A file systsem doesn't _need_ to do anything. It may chose to do
things, and the iomap based helpers might be useful for that. But
again, I'm still not getting what this document is even trying to
explain, as "to implement the method foo, use the iomap_foo" isn't
really helping anyone.
> +Converting filesystems from buffer-head to iomap guide
> +======================================================
If you want such a guide, please keep it in a separate file from the
iomap API documentation. I'd also suggest that you actually try such
a conversion first, as that might help shaping the documentation :)
> +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/
> +}}}
How does this belong into an iomap documentation? If LTPs dio is really
all that useful we should import it into xfstests, btw. I'm not sure it
is, though.
> +We try to document known issues that folks should be aware of with **iomap** here.
Who is "we"?
> + * DOC: Introduction
> + *
> + * iomap allows filesystems to sequentially iterate over byte addressable block
> + * ranges on an inode and apply operations to it.
> + *
> + * iomap grew out of the need to provide a modern block mapping abstraction for
> + * filesystems with the different IO access methods they support and assisting
> + * the VFS with manipulating files into the page cache. iomap helpers are
> + * provided for each of these mechanisms. However, block mapping is just one of
> + * the features of iomap, given iomap supports DAX IO for filesystems and also
> + * supports such the ``lseek``/``llseek`` ``SEEK_DATA``/``SEEK_HOLE``
> + * interfaces.
> + *
> + * 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. 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 replacement for **buffer-heads**.
> + */
I really don't want random blurbs and links like this in the main
header. If you want to ramble in a little howto that's fine, but the
main header is not the place for it.
Also please keep improvements to the header in a separate patch from
adding Documentation/ documents.
