On Tue 01-07-25 21:06:28, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>
> During the process of writing back folios, if
> mpage_map_and_submit_extent() exits the extent mapping loop due to an
> ENOSPC or ENOMEM error, it may result in stale data or filesystem
> inconsistency in environments where the block size is smaller than the
> folio size.
>
> When mapping a discontinuous folio in mpage_map_and_submit_extent(),
> some buffers may have already be mapped. If we exit the mapping loop
> prematurely, the folio data within the mapped range will not be written
> back, and the file's disk size will not be updated. Once the transaction
> that includes this range of extents is committed, this can lead to stale
> data or filesystem inconsistency.
>
> Fix this by submitting the current processing partially mapped folio.
>
> Suggested-by: Jan Kara <jack@xxxxxxx>
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Just one naming suggestion below:
> +/*
> + * This is used to submit mapped buffers in a single folio that is not fully
> + * mapped for various reasons, such as insufficient space or journal credits.
> + */
> +static int mpage_submit_buffers(struct mpage_da_data *mpd)
mpage_submit_buffers() sounds somewhat generic. How about
mpage_submit_partial_folio()?
Honza
> +{
> + struct inode *inode = mpd->inode;
> + struct folio *folio;
> + loff_t pos;
> + int ret;
> +
> + folio = filemap_get_folio(inode->i_mapping,
> + mpd->start_pos >> PAGE_SHIFT);
> + if (IS_ERR(folio))
> + return PTR_ERR(folio);
> + /*
> + * The mapped position should be within the current processing folio
> + * but must not be the folio start position.
> + */
> + pos = mpd->map.m_lblk << inode->i_blkbits;
> + if (WARN_ON_ONCE((folio_pos(folio) == pos) ||
> + !folio_contains(folio, pos >> PAGE_SHIFT)))
> + return -EINVAL;
> +
> + ret = mpage_submit_folio(mpd, folio);
> + if (ret)
> + goto out;
> + /*
> + * Update start_pos to prevent this folio from being released in
> + * mpage_release_unused_pages(), it will be reset to the aligned folio
> + * pos when this folio is written again in the next round. Additionally,
> + * do not update wbc->nr_to_write here, as it will be updated once the
> + * entire folio has finished processing.
> + */
> + mpd->start_pos = pos;
> +out:
> + folio_unlock(folio);
> + folio_put(folio);
> + return ret;
> +}
> +
> /*
> * mpage_map_and_submit_extent - map extent starting at mpd->lblk of length
> * mpd->len and submit pages underlying it for IO
> @@ -2411,8 +2452,16 @@ static int mpage_map_and_submit_extent(handle_t *handle,
> */
> if ((err == -ENOMEM) ||
> (err == -ENOSPC && ext4_count_free_clusters(sb))) {
> - if (progress)
> + /*
> + * We may have already allocated extents for
> + * some bhs inside the folio, issue the
> + * corresponding data to prevent stale data.
> + */
> + if (progress) {
> + if (mpage_submit_buffers(mpd))
> + goto invalidate_dirty_pages;
> goto update_disksize;
> + }
> return err;
> }
> ext4_msg(sb, KERN_CRIT,
> --
> 2.46.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
