On Sun, Jun 11, 2023 at 01:58:31AM -0400, Theodore Ts'o wrote: > On Mon, May 15, 2023 at 04:10:41PM +0530, Ritesh Harjani (IBM) wrote: > > mpage_submit_folio() was converted to take folio. Even though > > folio_size() in ext4 as of now is PAGE_SIZE, but it's better to > > remove that assumption which I am assuming is a missed left over from > > patch[1]. > > > > [1]: https://lore.kernel.org/linux-ext4/20230324180129.1220691-7-willy@xxxxxxxxxxxxx/ > > > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx> > > I didn't notice this right away, because the failure is not 100% > reliable, but this commit will sometimes cause "kvm-xfstests -c > ext4/encrypt generic/068" to crash. Reverting the patch fixes the > problem, so I plan to drop this patch from my tree. Hrm. Well, let's think about how this can go wrong: @@ -1885,7 +1885,7 @@ static int mpage_submit_folio(struct mpage_da_data *mpd, +struct folio *folio) len = folio_size(folio); if (folio_pos(folio) + len > size && !ext4_verity_in_progress(mpd->inode)) - len = size & ~PAGE_MASK; + len = size - folio_pos(folio); err = ext4_bio_write_folio(&mpd->io_submit, folio, len); if (!err) mpd->wbc->nr_to_write--; Just off-camera is: size = i_size_read(mpd->inode); Now, nothing is preventing i_size to be truncated to far below this folio, right? So if that happened before this patch, we'd write some randomly sized fragment of the page. Now we'll get a negative result ... which is assigned to size_t, so is exabytes in size. So do we care if we write a random fragment of a page after a truncate? If so, we should add: if (folio_pos(folio) >= size) return 0; /* Do we need to account nr_to_write? */ If we simply don't care that we're doing a spurious write, then we can do something like: - len = size & ~PAGE_MASK; + len = size & (len - 1);
