On 19/02/2024 22:58, Dave Chinner wrote:
On Mon, Feb 19, 2024 at 01:01:03PM +0000, John Garry wrote:
Add atomic write support as follows:
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 74e9e775f13d..12a75a252ca2 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -18,6 +18,42 @@
#include "blk-rq-qos.h"
#include "blk-throttle.h"
+static bool rq_straddles_atomic_write_boundary(struct request *rq,
+ unsigned int front,
+ unsigned int back)
+{
+ unsigned int boundary = queue_atomic_write_boundary_bytes(rq->q);
+ unsigned int mask, imask;
+ loff_t start, end;
+
+ if (!boundary)
+ return false;
+
+ start = rq->__sector << SECTOR_SHIFT;
+ end = start + rq->__data_len;
+
+ start -= front;
+ end += back;
+
+ /* We're longer than the boundary, so must be crossing it */
+ if (end - start > boundary)
+ return true;
+
+ mask = boundary - 1;
+
+ /* start/end are boundary-aligned, so cannot be crossing */
+ if (!(start & mask) || !(end & mask))
+ return false;
+
+ imask = ~mask;
+
+ /* Top bits are different, so crossed a boundary */
+ if ((start & imask) != (end & imask))
+ return true;
+
+ return false;
+}
I have no way of verifying this function is doing what it is
supposed to because it's function is undocumented. I have no idea
what the front/back variables are supposed to represent, and so no
clue if they are being applied properly.
I'll add proper function header documentation.
That said, it's also applying unsigned 32 bit mask variables to
signed 64 bit quantities and trying to do things like "high bit changed"
checks on the 64 bit variable. This just smells like a future
source of "large offsets don't work like we expected!" bugs.
I'll change variables to be unsigned and also all the same size.
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 06ea91e51b8b..176f26374abc 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -59,6 +59,13 @@ void blk_set_default_limits(struct queue_limits *lim)
lim->zoned = false;
lim->zone_write_granularity = 0;
lim->dma_alignment = 511;
+ lim->atomic_write_hw_max_sectors = 0;
+ lim->atomic_write_max_sectors = 0;
+ lim->atomic_write_hw_boundary_sectors = 0;
+ lim->atomic_write_hw_unit_min_sectors = 0;
+ lim->atomic_write_unit_min_sectors = 0;
+ lim->atomic_write_hw_unit_max_sectors = 0;
+ lim->atomic_write_unit_max_sectors = 0;
}
Seems to me this function would do better to just
memset(lim, 0, sizeof(*lim));
and then set all the non-zero fields.
Christoph responded about limits here and in
blk_queue_atomic_write_max_bytes(), so please let me know if still some
concerns.
Thanks,
John
