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(-)

-- 
2.39.2

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.

-- 
Chandan

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.

-- 
Chandan

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.

-- 
Chandan