On 04/04/2025 11:23, Ritesh Harjani (IBM) wrote:
John Garry<john.g.garry@xxxxxxxxxx> writes:
On 03/04/2025 19:22, Ritesh Harjani (IBM) wrote:
IMHO, This document seems to be updated a lot, to the point where I
think that it has too much detail.
Perhaps this [1] can change your mind? Just the second paragraph of this
article might be good reason to keep the design doc updated with latest
changes in the iomap code.
[1]:https://urldefense.com/v3/__https://lwn.net/Articles/935934/__;!!
ACWV5N9M2RV99hQ!
M5YtnH5eBpf0C629QX_zsHZjxSMfWBW8svEup_qNhkg2ie5uqB81lAEO_3DR2pKKSYqUZgLGXiUyQUqi_mjMeZc$
I am happy to see documentation, but I think that there is too much
fine-grained detail in this case.
For my large atomic writes support for XFS series, I am looking at this
document and thinking that I need to update it again as I am introducing
a new error code for iomap_dio_rw(). I don't want to have to update the
document every time I touch iomap or related code.
* **IOMAP_F_PRIVATE**: Starting with this value, the upper bits can
be set by the filesystem for its own purposes.
Is this comment now out of date according to your change in 923936efeb74?
Yup. Thanks for catching that. I am thinking we can update this to:
* **IOMAP_F_PRIVATE**: This flag is reserved for filesystem private use.
Currently only gfs2 uses this for implementing buffer head metadata
boundary.
do we really want to update the doc (or even the iomap.h) if some other
FS uses this flag? I don't think so.
This is done by gfs2 to avoid fetching the next mapping as
otherwise it could likely incur an additional I/O to fetch the
indirect metadata block.
If this looks good to others too I will update this in the v2.
Though, I now wonder whether gfs2 can also just use the IOMAP_F_BOUNDARY
flag instead of using IOMAP_F_PRIVATE?
I'm not sure
@@ -250,6 +255,11 @@ The fields are as follows:
block assigned to it yet and the file system will do that in the bio
submission handler, splitting the I/O as needed.
+ * **IOMAP_F_ATOMIC_BIO**: Indicates that write I/O must be submitted
+ with the ``REQ_ATOMIC`` flag set in the bio.
This is effectively the same comment as iomap.h
Filesystems need to set
+ this flag to inform iomap that the write I/O operation requires
+ torn-write protection based on HW-offload mechanism.
Personally I think that this is obvious. If not, the reader should check
the xfs and ext4 example in the code.
It's just my opinion, but sometimes including examples of how such flags
are used in the code - within the design document, can help the reader
better understand their context and purpose.
Sure, but you need to consider the burden of maintaining this document
and whether it is even 100% accurate always.
