On 23/04/2025 06:18, Luis Chamberlain wrote:
On Tue, Apr 22, 2025 at 07:08:32AM +0100, John Garry wrote:
On 21/04/2025 22:18, Luis Chamberlain wrote:
/*
+ * The retry mechanism is based on the ->iomap_begin method returning
+ * -ENOPROTOOPT, which would be when the REQ_ATOMIC-based write is not
+ * possible. The REQ_ATOMIC-based method typically not be possible if
+ * the write spans multiple extents or the disk blocks are misaligned.
+ */
+ if (ret == -ENOPROTOOPT && dops == &xfs_direct_write_iomap_ops) {
Based on feedback from LSFMM, due to the performance variaibility this
can introduce, it sounded like some folks would like to opt-in to not
have a software fallback and just require an error out.
Could an option be added to not allow the software fallback?
I still don't see the use in this.
Its not the use, its the concern for underdeterminism in performance.
Sure, we don't offer RT performance guarantees, but what does?
So consider userspace wants to write something atomically and we fail as a
HW-based atomic write is not possible.
Sounds like a terrible predicant for those that want hw atomics and
reliability for it.
Well from our MySQL testing performance is good.
What is userspace going to do next?
It would seem that would depend on their analysis on the number of
software fallbacks where a software atomic based solution is used and
the impact on performance.
sorry, but I don't understand this
I heard something like "if HW-based atomics are not possible, then something
has not been configured properly for the FS" - that something would be
extent granularity and alignment, but we don't have a method to ensure this.
That is the whole point of having a FS fallback.
We do with LBS.
Sure, but not everyone wants LBS
Its perfectly deterministic to be aligned with a sector
size matching the block size, even for metadata writes.
If so, then I think the next patch would also need updating.
Or are you suggesting that without the software fallback atomic writes
greater than fs block size are not possible?
Yes, as XFS has no method to guarantee extent granularity and alignment.
Ah, I think the documentation for this featuer should make this clear,
it was not clear up to this point in patch review.
ok, that can be added
