| Message ID | 20240320110548.2200662-1-yi.zhang@huaweicloud.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | xfs/iomap: fix non-atomic clone operation and don't update size when zeroing range post eof | expand |
On Wed, Mar 20, 2024 at 07:05:39 PM +0800, Zhang Yi wrote: > Changes since v3: > - Improve some git message comments and do some minor code cleanup, no > logic changes. > > Changes since v2: > - Merge the patch for dropping of xfs_convert_blocks() and the patch > for modifying xfs_bmapi_convert_delalloc(). > - Reword the commit message of the second patch. > > Changes since v1: > - Make xfs_bmapi_convert_delalloc() to allocate the target offset and > drop the writeback helper xfs_convert_blocks(). > - Don't use xfs_iomap_write_direct() to convert delalloc blocks for > zeroing posteof case, use xfs_bmapi_convert_delalloc() instead. > - Fix two off-by-one issues when converting delalloc blocks. > - Add a separate patch to drop the buffered write failure handle in > zeroing and unsharing. > - Add a comments do emphasize updating i_size should under folio lock. > - Make iomap_write_end() to return a boolean, and do some cleanups in > buffered write begin path. > > This patch series fix a problem of exposing zeroed data on xfs since the > non-atomic clone operation. This problem was found while I was > developing ext4 buffered IO iomap conversion (ext4 is relying on this > fix [1]), the root cause of this problem and the discussion about the > solution please see [2]. After fix the problem, iomap_zero_range() > doesn't need to update i_size so that ext4 can use it to zero partial > block, e.g. truncate eof block [3]. > > [1] https://lore.kernel.org/linux-ext4/20240127015825.1608160-1-yi.zhang@huaweicloud.com/ > [2] https://lore.kernel.org/linux-ext4/9b0040ef-3d9d-6246-4bdd-82b9a8f55fa2@huaweicloud.com/ > [3] https://lore.kernel.org/linux-ext4/9c9f1831-a772-299b-072b-1c8116c3fb35@huaweicloud.com/ > > Thanks, > Yi. > > Zhang Yi (9): > xfs: match lock mode in xfs_buffered_write_iomap_begin() > xfs: make the seq argument to xfs_bmapi_convert_delalloc() optional > xfs: make xfs_bmapi_convert_delalloc() to allocate the target offset > xfs: convert delayed extents to unwritten when zeroing post eof blocks > iomap: drop the write failure handles when unsharing and zeroing > iomap: don't increase i_size if it's not a write operation > iomap: use a new variable to handle the written bytes in > iomap_write_iter() > iomap: make iomap_write_end() return a boolean > iomap: do some small logical cleanup in buffered write > Hi all, I have picked up this patchset for inclusion into XFS' 6.10-rc1 patch queue. Please let me know if there are any objections.
On Wed, Apr 17, 2024 at 10:12:10AM +0530, Chandan Babu R wrote: > On Wed, Mar 20, 2024 at 07:05:39 PM +0800, Zhang Yi wrote: > > Changes since v3: > > - Improve some git message comments and do some minor code cleanup, no > > logic changes. > > > > Changes since v2: > > - Merge the patch for dropping of xfs_convert_blocks() and the patch > > for modifying xfs_bmapi_convert_delalloc(). > > - Reword the commit message of the second patch. > > > > Changes since v1: > > - Make xfs_bmapi_convert_delalloc() to allocate the target offset and > > drop the writeback helper xfs_convert_blocks(). > > - Don't use xfs_iomap_write_direct() to convert delalloc blocks for > > zeroing posteof case, use xfs_bmapi_convert_delalloc() instead. > > - Fix two off-by-one issues when converting delalloc blocks. > > - Add a separate patch to drop the buffered write failure handle in > > zeroing and unsharing. > > - Add a comments do emphasize updating i_size should under folio lock. > > - Make iomap_write_end() to return a boolean, and do some cleanups in > > buffered write begin path. > > > > This patch series fix a problem of exposing zeroed data on xfs since the > > non-atomic clone operation. This problem was found while I was > > developing ext4 buffered IO iomap conversion (ext4 is relying on this > > fix [1]), the root cause of this problem and the discussion about the > > solution please see [2]. After fix the problem, iomap_zero_range() > > doesn't need to update i_size so that ext4 can use it to zero partial > > block, e.g. truncate eof block [3]. > > > > [1] https://lore.kernel.org/linux-ext4/20240127015825.1608160-1-yi.zhang@huaweicloud.com/ > > [2] https://lore.kernel.org/linux-ext4/9b0040ef-3d9d-6246-4bdd-82b9a8f55fa2@huaweicloud.com/ > > [3] https://lore.kernel.org/linux-ext4/9c9f1831-a772-299b-072b-1c8116c3fb35@huaweicloud.com/ > > > > Thanks, > > Yi. > > > > Zhang Yi (9): > > xfs: match lock mode in xfs_buffered_write_iomap_begin() > > xfs: make the seq argument to xfs_bmapi_convert_delalloc() optional > > xfs: make xfs_bmapi_convert_delalloc() to allocate the target offset > > xfs: convert delayed extents to unwritten when zeroing post eof blocks > > iomap: drop the write failure handles when unsharing and zeroing > > iomap: don't increase i_size if it's not a write operation > > iomap: use a new variable to handle the written bytes in > > iomap_write_iter() > > iomap: make iomap_write_end() return a boolean > > iomap: do some small logical cleanup in buffered write > > > > Hi all, > > I have picked up this patchset for inclusion into XFS' 6.10-rc1 patch > queue. Please let me know if there are any objections. It'd be nice if I could take the iomap patches into the vfs.iomap tree that you can then pull from if you depend on it.. There's already some cleanups in there. Sounds ok?
On Wed, Apr 17, 2024 at 01:40:36 PM +0200, Christian Brauner wrote: > On Wed, Apr 17, 2024 at 10:12:10AM +0530, Chandan Babu R wrote: >> On Wed, Mar 20, 2024 at 07:05:39 PM +0800, Zhang Yi wrote: >> > Changes since v3: >> > - Improve some git message comments and do some minor code cleanup, no >> > logic changes. >> > >> > Changes since v2: >> > - Merge the patch for dropping of xfs_convert_blocks() and the patch >> > for modifying xfs_bmapi_convert_delalloc(). >> > - Reword the commit message of the second patch. >> > >> > Changes since v1: >> > - Make xfs_bmapi_convert_delalloc() to allocate the target offset and >> > drop the writeback helper xfs_convert_blocks(). >> > - Don't use xfs_iomap_write_direct() to convert delalloc blocks for >> > zeroing posteof case, use xfs_bmapi_convert_delalloc() instead. >> > - Fix two off-by-one issues when converting delalloc blocks. >> > - Add a separate patch to drop the buffered write failure handle in >> > zeroing and unsharing. >> > - Add a comments do emphasize updating i_size should under folio lock. >> > - Make iomap_write_end() to return a boolean, and do some cleanups in >> > buffered write begin path. >> > >> > This patch series fix a problem of exposing zeroed data on xfs since the >> > non-atomic clone operation. This problem was found while I was >> > developing ext4 buffered IO iomap conversion (ext4 is relying on this >> > fix [1]), the root cause of this problem and the discussion about the >> > solution please see [2]. After fix the problem, iomap_zero_range() >> > doesn't need to update i_size so that ext4 can use it to zero partial >> > block, e.g. truncate eof block [3]. >> > >> > [1] https://lore.kernel.org/linux-ext4/20240127015825.1608160-1-yi.zhang@huaweicloud.com/ >> > [2] https://lore.kernel.org/linux-ext4/9b0040ef-3d9d-6246-4bdd-82b9a8f55fa2@huaweicloud.com/ >> > [3] https://lore.kernel.org/linux-ext4/9c9f1831-a772-299b-072b-1c8116c3fb35@huaweicloud.com/ >> > >> > Thanks, >> > Yi. >> > >> > Zhang Yi (9): >> > xfs: match lock mode in xfs_buffered_write_iomap_begin() >> > xfs: make the seq argument to xfs_bmapi_convert_delalloc() optional >> > xfs: make xfs_bmapi_convert_delalloc() to allocate the target offset >> > xfs: convert delayed extents to unwritten when zeroing post eof blocks >> > iomap: drop the write failure handles when unsharing and zeroing >> > iomap: don't increase i_size if it's not a write operation >> > iomap: use a new variable to handle the written bytes in >> > iomap_write_iter() >> > iomap: make iomap_write_end() return a boolean >> > iomap: do some small logical cleanup in buffered write >> > >> >> Hi all, >> >> I have picked up this patchset for inclusion into XFS' 6.10-rc1 patch >> queue. Please let me know if there are any objections. > > It'd be nice if I could take the iomap patches into the vfs.iomap tree > that you can then pull from if you depend on it.. There's already some > cleanups in there. Sounds ok? Yes, that works for me. I will pick only those patches that are specific to XFS i.e. patches numbered 1 to 4.
On Wed, 20 Mar 2024 19:05:39 +0800, Zhang Yi wrote: > Changes since v3: > - Improve some git message comments and do some minor code cleanup, no > logic changes. > > Changes since v2: > - Merge the patch for dropping of xfs_convert_blocks() and the patch > for modifying xfs_bmapi_convert_delalloc(). > - Reword the commit message of the second patch. > > [...] @Chandan, since the bug has been determined to be in the xfs specific changes for this I've picked up the cleanup patches into vfs.iomap. If you need to rely on that branch I can keep it stable. --- Applied to the vfs.iomap branch of the vfs/vfs.git tree. Patches in the vfs.iomap branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.iomap [5/9] iomap: drop the write failure handles when unsharing and zeroing https://git.kernel.org/vfs/vfs/c/89c6c1d91ab2 [6/9] iomap: don't increase i_size if it's not a write operation https://git.kernel.org/vfs/vfs/c/943bc0882ceb [7/9] iomap: use a new variable to handle the written bytes in iomap_write_iter() https://git.kernel.org/vfs/vfs/c/1a61d74932d4 [8/9] iomap: make iomap_write_end() return a boolean https://git.kernel.org/vfs/vfs/c/815f4b633ba1 [9/9] iomap: do some small logical cleanup in buffered write https://git.kernel.org/vfs/vfs/c/e1f453d4336d
On Thu, Apr 25, 2024 at 02:25:47 PM +0200, Christian Brauner wrote: > On Wed, 20 Mar 2024 19:05:39 +0800, Zhang Yi wrote: >> Changes since v3: >> - Improve some git message comments and do some minor code cleanup, no >> logic changes. >> >> Changes since v2: >> - Merge the patch for dropping of xfs_convert_blocks() and the patch >> for modifying xfs_bmapi_convert_delalloc(). >> - Reword the commit message of the second patch. >> >> [...] > > @Chandan, since the bug has been determined to be in the xfs specific changes > for this I've picked up the cleanup patches into vfs.iomap. If you need to rely > on that branch I can keep it stable. I am sorry about the late reply. I somehow missed this mail. I will pick up the XFS specific patches now.
Changes since v3: - Improve some git message comments and do some minor code cleanup, no logic changes. Changes since v2: - Merge the patch for dropping of xfs_convert_blocks() and the patch for modifying xfs_bmapi_convert_delalloc(). - Reword the commit message of the second patch. Changes since v1: - Make xfs_bmapi_convert_delalloc() to allocate the target offset and drop the writeback helper xfs_convert_blocks(). - Don't use xfs_iomap_write_direct() to convert delalloc blocks for zeroing posteof case, use xfs_bmapi_convert_delalloc() instead. - Fix two off-by-one issues when converting delalloc blocks. - Add a separate patch to drop the buffered write failure handle in zeroing and unsharing. - Add a comments do emphasize updating i_size should under folio lock. - Make iomap_write_end() to return a boolean, and do some cleanups in buffered write begin path. This patch series fix a problem of exposing zeroed data on xfs since the non-atomic clone operation. This problem was found while I was developing ext4 buffered IO iomap conversion (ext4 is relying on this fix [1]), the root cause of this problem and the discussion about the solution please see [2]. After fix the problem, iomap_zero_range() doesn't need to update i_size so that ext4 can use it to zero partial block, e.g. truncate eof block [3]. [1] https://lore.kernel.org/linux-ext4/20240127015825.1608160-1-yi.zhang@huaweicloud.com/ [2] https://lore.kernel.org/linux-ext4/9b0040ef-3d9d-6246-4bdd-82b9a8f55fa2@huaweicloud.com/ [3] https://lore.kernel.org/linux-ext4/9c9f1831-a772-299b-072b-1c8116c3fb35@huaweicloud.com/ Thanks, Yi. Zhang Yi (9): xfs: match lock mode in xfs_buffered_write_iomap_begin() xfs: make the seq argument to xfs_bmapi_convert_delalloc() optional xfs: make xfs_bmapi_convert_delalloc() to allocate the target offset xfs: convert delayed extents to unwritten when zeroing post eof blocks iomap: drop the write failure handles when unsharing and zeroing iomap: don't increase i_size if it's not a write operation iomap: use a new variable to handle the written bytes in iomap_write_iter() iomap: make iomap_write_end() return a boolean iomap: do some small logical cleanup in buffered write fs/iomap/buffered-io.c | 105 ++++++++++++++++++++++----------------- fs/xfs/libxfs/xfs_bmap.c | 40 +++++++++++++-- fs/xfs/xfs_aops.c | 54 ++++++-------------- fs/xfs/xfs_iomap.c | 39 +++++++++++++-- 4 files changed, 144 insertions(+), 94 deletions(-)