Re: [PATCH v4 18/23] ext4: wait for ordered I/O in the iomap buffered I/O path
From: Zhang Yi
Date: Fri Jul 03 2026 - 22:34:39 EST
On 7/2/2026 10:20 PM, Jan Kara wrote:
On Tue 30-06-26 20:06:55, Zhang Yi wrote:
On 6/25/2026 5:42 PM, Jan Kara wrote:
On Mon 22-06-26 11:32:16, Zhang Yi wrote:
On 6/18/2026 9:48 PM, Jan Kara wrote:
On Mon 11-05-26 15:23:38, Zhang Yi wrote:
From: Zhang Yi <yi.zhang@xxxxxxxxxx>
For append writes, wait for ordered I/O to complete before updating
i_disksize. This ensures that zeroed data is flushed to disk before the
metadata update, preventing stale data from being exposed during
unaligned post-EOF append writes.
Suggested-by: Jan Kara <jack@xxxxxxx>
Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
Frankly, this all looks too complex to me. Plus your are adding 32-bytes to
struct ext4_inode_info which isn't great either. Why don't you just do
filemap_fdatawait() for the byte at old i_disksize and be done with it?
I believe we have to simplify this. All this complexity (and thus
maintenance burden) across several patches for the corner case of zeroing
tail block on extention is in my opinion difficult to justify.
Honza
Hi, Jan!
Thanks for the review. I understand the concern about complexity and the
32-byte increase to ext4_inode_info. I tried using
filemap_fdatawait_range() as you suggested, but found two issues where
this solution doesn't work.
1. ioend worker deadlock
Since worker concurrency resources are limited, we cannot wait for
another ioend worker to complete within one ioend worker with the same
work_struct. If the worker calls
filemap_fdatawait_range(byte_at_old_disksize) to wait for the zeroed
block's folio writeback to complete, it sleeps holding the only worker
slot. If the folio contains blocks requiring extent conversion, its
writeback bit is cleared by iomap_finish_ioends() running inside
another worker -- which can only run after the current worker finishes
its batch.
Concretely:
- Worker W1 processes ioend A, calls filemap_fdatawait_range() on
the old EOF byte, sleeps.
- The zeroed data is in ioend B. bio_endio defers it to
i_iomap_ioend_list and calls queue_work().
- queue_work() on i_iomap_ioend_work is idempotent: it returns false
because the work is currently executing (even though sleeping).
- ioend B sits in the list, never gets processed.
- The folio writeback bit is only cleared by processing ioend B.
- W1 sleeps forever -> deadlock.
Yes, good point. We cannot wait for folio writeback completion from end_io
processing for another folio.
Therefore, I think we have to put the wakeup logic in
ext4_iomap_end_bio() that runs in interrupt context without consuming
a worker thread. The ordered range tracking and wait queue are what
make that possible.
2. Truncate-up needs an accurate state query
In the follow-up patch 19, ext4_set_inode_size() must make a precise
decision when updating i_disksize during truncate up.
This needs a state query: "is there ordered zero I/O in flight right
now?" If yes, the i_disksize update is deferred to
ext4_iomap_wb_update_disksize(is_ordered=true), which advances
i_disksize to i_size when the ordered I/O completes. If no, we must
advance i_disksize immediately, otherwise we will lose the updating
forever.
Therefore, we need to track the state of the ordered range. Simply
using filemap_fdatawait_range() doesn't work. i_ordered_len serves as
a maintained state flag that both the ioend completion path and the
setattr path can read atomically without sleeping.
Suggestions?
I see. Thanks for explanation. I went back to our discussion back from
February to remind myself about the constraints on the tail block zeroing
and the i_disksize update mechanism. And in the light of complexity of the
current mechanism, I think we've discarded the following possibility too
easily:
* On file extend / truncate up just zero tail folio in the page cache, mark
it dirty, keep i_disksize at old value, update i_size to the new value,
add inode to orphan list.
If the i_disksize was block aligned (and so we skip zeroing), we just
update i_disksize rightaway.
* In io end processing if the folio for which we end io has a block which
straddles i_disksize, we update i_disksize to current i_size. We defer
removing inode from orphan list e.g. to file close time (doing it from
end_io processing is problematic locking wise as we need i_rwsem for it).
This is a very simple scheme with very good performace. It makes sure stale
data in the tail block cannot be seen on disk after a crash.
Hmm, I think this solution has a problem. Since i_disksize is advanced
to i_size only when the ioend covering the i_disksize block completes,
we must ensure that i_size is updated before the zeroed folio's
writeback completes. Otherwise the ioend worker reads the old i_size,
advances i_disksize to the old value (a no-op), and i_disksize stays
stale forever after i_size is updated.
Currently, neither truncate-up nor file-extend paths guarantee this
ordering -- in both, ext4_block_zero_eof() runs before the i_size
update.
Good point, that needs to be handled.
For truncate-up and fallocate, we might be able to split
ext4_update_inode_size() / ext4_set_inode_size() so that i_size is
updated before ext4_block_zero_eof(), and then decide whether to
update i_disksize based on whether i_disksize is block-aligned. I'm
not yet sure what side effects this reordering would have, but I feel
uneasy about this.
The append-write case is hard to solve. We can't know the final i_size
before the write completes, so we can't update i_size ahead of
ext4_block_zero_eof(). When the zeroed folio is later written back, it's
treated as an ordinary overwriting write -- i_disksize is not advanced.
And when the subsequent buffered-write data is written back, i_disksize
still isn't advanced either. The size update is lost.
One thing we need to do is to perform i_size updates in append write,
truncate, etc. under i_data_sem (similarly to i_disksize updates) so that
we can properly synchronize i_size + i_disksize updates from IO completion
and these paths.
How we could then handle the races between writeback of folio straddling
i_disksize and the i_size updates from the i_size extending paths is that
we'd set EXT4_STATE_DISKSIZE_GROW_PENDING bit when zeroing the folio
straddling i_disksize and clear this (under i_data_sem) in IO completion
when updating i_disksize to i_size. And when setting new i_size after
append write, truncate up, etc. we will also update i_disksize when this
state bit is not set (again all happening under i_data_sem). This should
also automatically include the case where i_disksize was block aligned and
thus no zeroing was needed and we need to update i_disksize together with
i_size. Sounds good?
Honza
Ha, yeah. This overall looks good to me now. Your solution doesn't look
significantly different from mine currently implementation. My approach
uses i_ordered_len to precisely track the pending state of whether a
zeroed folio is awaiting submission, while you choose to use
EXT4_STATE_DISKSIZE_GROW_PENDING to represent this, which completely
eliminates the 32-byte bloat in ext4_inode_info.
However, I'd prefer we should still be able to keep the IO-completion
waiting logic instead of adding the orphan list. We can precisely
determine whether an IO is a "cross-i_disksize IO" by checking whether
its position straddles i_disksize, and when it completes, wake up the
worker waiting for it in interrupt context -- this won't deadlock (bio
end_io doesn't consume a worker thread). Besides, Since we plan to use
the EXT4_STATE_DISKSIZE_GROW_PENDING bit, the waiting doesn't require a
dedicated wait queue; wait_on_bit / wake_up_bit(&ei->i_state_flags,
EXT4_STATE_DISKSIZE_GROW_PENDING) should suffice.
Moreover, if we use the orphan list approach, it seems difficult to
decide when to release the orphan list entries. I'm concerned that
there may be concurrency issues that would need careful consideration
(For example, if the orphan list is deleted when the file is closed,
then after the last opener closes the file, is it necessary to wait for
synchronous writeback? If there is concurrent file expansion during the
writeback process, are there concurrency issues?) The original waiting
scheme is more straightforward to me.
In short, For v5, I think we should:
1) Track state via EXT4_STATE_DISKSIZE_GROW_PENDING instead of checking
whether i_ordered_len is 0;
2) Determine the I/O type during writeback by checking i_disksize
instead of the i_ordered_lblk parameter;
3) No longer use a dedicated wait queue; instead wait on
EXT4_STATE_DISKSIZE_GROW_PENDING.
Thereforer, I think this solution won't simplify the logic much compared
to my current one; it will likely still require changes in quite a few
patches. The benefit is saving memory. I can't think of any other
potential issues for now, So I'm glad to have a try.
In addition, I'll need to address a few more details in v5:
1. Following earlier discussions with Ojaswin, I plan to drop the
patch 07 "ext4: submit zeroed post-EOF data immediately in the iomap
buffered I/O path" in the next version -- no longer submitting I/O
immediately after zeroing the EOF block, to improve concurrency.
Then, in ext4_iomap_writepages() we check the writeback range:
if it behind the zeroed folio, we explicitly submit the zeroed folio
to prevent the appending I/O from being wait too long.
2. We also need to handle the case where the zeroed folio is
truncated before being written back. In that case, i_disksize must
be updated directly since the folio no longer has a chance to be
written back.
3. In the SYNC mode of fallocate and zero range, the zeroed folio
needs to be submitted synchronously as well; otherwise i_disksize
won't be updated immediately (which would violate SYNC semantics).
What do you think?
Besides, I've been revisiting why the iomap buffered I/O series disables
data=ordered mode these days. If we can directly solve the deadlock
problem in data=ordered mode, things would be much simpler. However,
IIRC, I recall that Ted previously mentioned planning to deprecate the
data=ordered mode in the future, so I'm not sure whether exploring this
direction is worthwhile. In any case, let me share my thoughts:
The root cause of this deadlock is that, during writeback, we cannot
start a new handle/transaction without stalling on the currently
committing transaction (T_N) to finish. Please see the Patch 07 with the
deadlock call graph:
https://github.com/zhangyi089/linux/commit/c1a0e32ebf134411edb600b9b602b4af9420d25d
When writeback tries to start T_{N+1}, it has two stalls involved:
[ T_0 | T_1 | ... | T_N-1 | T_N ]
1. Waiting for the T_N to enter T_FLUSH if it has start committing, so a
new running transaction can be created. This is one-directional
waiting, not part of the deadlock cycle.
2. Waiting for log space, which is where the deadlock cycle below
closes.
T_{N+1} start -> waits for -> T_0/1/... checkpoint completion
T_0 checkpoint -> waits for -> T_N commit completion if it
contians the same bh
T_N commit -> waits for -> ordered data writeback
ordered writeback = T_{N+1} start path -> (cycle)
Only case 2 is the actual deadlock. So what I'm thinking is could we try
to break the cycle at the T_0 checkpoint -> T_N commit edge? That is
when checkpoint hits a bh that T_N has adopted, write the pre-T_N
contents to the original on-disk location instead of waiting for T_N to
commit. To avoid excessive copying, this requires:
- Identifying T_N as the last transaction that can start without
blocking on log space reclamation — once T_N starts, the remaining
log space is enough to fit T_N's own handle credits, but not enough to
fit another full j_max_transaction_buffers after that.
- When T_N adopts a bh still on an older checkpoint list, snapshot the
bh contents at adoption time, so the older checkpoint can write that
snapshot to disk independently of T_N's commit progress.
[ T_0 | T_1 | ... | T_N-1 | T_N ]
|<-- -->| -> T_N
(j_max_transaction_buffers)
1. Mark T_N as the last TX that started without obstruction. When a new
handle joins a running transaction, evaluate whether the remaining
log space, after subtracting this handle's credits, can still fit a
full j_max_transaction_buffers. If not, the current transaction is
committed immediately and the handle joins the next one, which
becomes T_N — the last transaction that can start without log-space
reclamation, T_N is then marked.
2. Snapshotting on adoption. In do_get_write_access(), when the current
handle's transaction is T_N and the bh is still on an older
checkpoint list, copy the bh contents into a new
jh->b_checkpoint_data pointer(like b_frozen_data does);
3. Consuming the snapshot at checkpoint. In jbd2_log_do_checkpoint(),
the current branch that handles an adopted bh by starting and waiting
for T_N's commit is extended: if b_checkpoint_data is present, write
it to the original block location and remove the bh from the
checkpoint list — without waiting for T_N to commit.
4. Releasing b_checkpoint_data. Freed when the bh is unlinked from its
checkpoint list or T_N is finished to commit.
This keeps the snapshot granularity at a minimum, only T_N's adopted
buffers get the extra copy. When metadata changes are light, T_N rarely
adopts such buffers, and the cost is unnoticeable; when metadata changes
are heavy , the dominant cost is the metadata writeback itself, and the
extra in-memory copy is unlikely to be the bottleneck. However, the
downside is that there may be some memory overhead, especially in the
metadata-intensive cases.
Another possible optimization: at T_N start, opportunistically try to
push the checkpoint tail. If enough space is reclaimed that another full
j_max_transaction_buffers fits after T_N's handle credits, T_N is no
longer "last" and the snapshotting can be skipped entirely.
If this scheme works, I think we should be able to completely remove the
limitation that writeback cannot start a handle in data=ordered mode.
This would also benefit the buffer_head path — perhaps we could
deprecate the reserved handle mechanism.
This is just a preliminary idea. What do you think of this approach?
Thanks,
Yl.