| Message ID | tencent_87F5A404ED65B0B1349C60E830C485716705@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ext4:Fix WARNING in ext4_journalled_invalidate_folio | expand |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The WARNING in ext4_journalled_invalidate_folio can still be triggered from ext4_write_begin on error paths.
--
commit 83d8cc4138093c0b6b19db042eb3a85e72ca5b68
Author: shuo chen <1289151713@qq.com>
ext4:Fix WARNING in ext4_journalled_invalidate_folio
This commit fixes a warning in ext4_journalled_invalidate_folio() by
calling ext4_wait_for_tail_page_commit() before truncating a failed write
in ext4_journalled_write_end(). This prevents invalidating the folio while
it is being committed.
Suggested-by: Jan Kara <jack@suse.cz>
Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Closes:https://lore.kernel.org/linux-ext4/CA+0ovCgxFN3TLpdS_O_Z48sARuQ-WR-JLDCqQWEmd9ELtkzddA@mail.gmail.com/T/
Signed-off-by: shuo chen <1289151713@qq.com>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 26f0f9714f03c..960a049a67bd2 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
[ ... ]
> @@ -1597,6 +1598,7 @@ static int ext4_journalled_write_end(const struct kiocb *iocb,
> if (!ret)
> ret = ret2;
> if (pos + len > inode->i_size && !verity) {
> + ext4_wait_for_tail_page_commit(inode);
> ext4_truncate_failed_write(inode);
> /*
> * If truncate failed early the inode might still be
[Severity: High]
This isn't a bug introduced by this patch, but it appears the same issue
might still exist on the error paths in ext4_write_begin().
When a write fails (e.g., due to ENOSPC or block allocation failure) and
extends i_size on a data=journal file, does ext4_write_begin() also need
to wait for the tail page commit?
fs/ext4/inode.c:ext4_write_begin() {
...
ext4_journal_stop(handle);
if (extended) {
ext4_truncate_failed_write(inode);
...
}
If ext4_truncate_failed_write() attempts to invalidate the folio without
waiting, jbd2_journal_invalidate_folio() could return -EBUSY because the
buffer is still committing. Could this leave the page cache in an
inconsistent state and trigger the WARN_ON in
ext4_journalled_invalidate_folio()?
On Tue 15-09-26 00:41:44, shuo chen wrote: > Thanks to Jack's suggestion. > Call the ext4_wait_for_tail_page_commit() helper before > ext4_truncate_failed_write() from ext4_journalled_write_end(), > which avoids invalidating the folio while it is being committed. > > Suggested-by: Jan Kara <jack@suse.cz> > Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu> > Closes:https://lore.kernel.org/linux-ext4/CA+0ovCgxFN3TLpdS_O_Z48sARuQ-WR-JLDCqQWEmd9ELtkzddA@mail.gmail.com/T/ > Signed-off-by: shuo chen <1289151713@qq.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/inode.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index c2c2d6ac7f3d..10a89bbb994a 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -56,6 +56,7 @@ static void ext4_journalled_zero_new_buffers(handle_t *handle, > struct inode *inode, > struct folio *folio, > unsigned from, unsigned to); > +static void ext4_wait_for_tail_page_commit(struct inode *inode); > > static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw, > struct ext4_inode_info *ei) > @@ -1605,6 +1606,7 @@ static int ext4_journalled_write_end(const struct kiocb *iocb, > if (!ret) > ret = ret2; > if (pos + len > inode->i_size && !verity) { > + ext4_wait_for_tail_page_commit(inode); > ext4_truncate_failed_write(inode); > /* > * If truncate failed early the inode might still be > -- > 2.55.0 >
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index c2c2d6ac7f3d..10a89bbb994a 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -56,6 +56,7 @@ static void ext4_journalled_zero_new_buffers(handle_t *handle, struct inode *inode, struct folio *folio, unsigned from, unsigned to); +static void ext4_wait_for_tail_page_commit(struct inode *inode); static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw, struct ext4_inode_info *ei) @@ -1605,6 +1606,7 @@ static int ext4_journalled_write_end(const struct kiocb *iocb, if (!ret) ret = ret2; if (pos + len > inode->i_size && !verity) { + ext4_wait_for_tail_page_commit(inode); ext4_truncate_failed_write(inode); /* * If truncate failed early the inode might still be
Thanks to Jack's suggestion. Call the ext4_wait_for_tail_page_commit() helper before ext4_truncate_failed_write() from ext4_journalled_write_end(), which avoids invalidating the folio while it is being committed. Suggested-by: Jan Kara <jack@suse.cz> Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu> Closes:https://lore.kernel.org/linux-ext4/CA+0ovCgxFN3TLpdS_O_Z48sARuQ-WR-JLDCqQWEmd9ELtkzddA@mail.gmail.com/T/ Signed-off-by: shuo chen <1289151713@qq.com> --- fs/ext4/inode.c | 2 ++ 1 file changed, 2 insertions(+)