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/[email protected]/
[2] https://lore.kernel.org/linux-ext4/[email protected]/
[3] https://lore.kernel.org/linux-ext4/[email protected]/
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
From: Zhang Yi <[email protected]>
In iomap_write_iter(), the status variable used to receive the return
value from iomap_write_end() is confusing, replace it with a new written
variable to represent the written bytes in each cycle, no logic changes.
Signed-off-by: Zhang Yi <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
fs/iomap/buffered-io.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index e9112dc78d15..291648c61a32 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -851,7 +851,7 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
loff_t length = iomap_length(iter);
size_t chunk = PAGE_SIZE << MAX_PAGECACHE_ORDER;
loff_t pos = iter->pos;
- ssize_t written = 0;
+ ssize_t total_written = 0;
long status = 0;
struct address_space *mapping = iter->inode->i_mapping;
unsigned int bdp_flags = (iter->flags & IOMAP_NOWAIT) ? BDP_ASYNC : 0;
@@ -862,6 +862,7 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
size_t offset; /* Offset into folio */
size_t bytes; /* Bytes to write to folio */
size_t copied; /* Bytes copied from user */
+ size_t written; /* Bytes have been written */
bytes = iov_iter_count(i);
retry:
@@ -906,7 +907,7 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
flush_dcache_folio(folio);
copied = copy_folio_from_iter_atomic(folio, offset, bytes, i);
- status = iomap_write_end(iter, pos, bytes, copied, folio);
+ written = iomap_write_end(iter, pos, bytes, copied, folio);
/*
* Update the in-memory inode size after copying the data into
@@ -916,22 +917,22 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
* unlock and release the folio.
*/
old_size = iter->inode->i_size;
- if (pos + status > old_size) {
- i_size_write(iter->inode, pos + status);
+ if (pos + written > old_size) {
+ i_size_write(iter->inode, pos + written);
iter->iomap.flags |= IOMAP_F_SIZE_CHANGED;
}
- __iomap_put_folio(iter, pos, status, folio);
+ __iomap_put_folio(iter, pos, written, folio);
if (old_size < pos)
pagecache_isize_extended(iter->inode, old_size, pos);
- if (status < bytes)
- iomap_write_failed(iter->inode, pos + status,
- bytes - status);
- if (unlikely(copied != status))
- iov_iter_revert(i, copied - status);
+ if (written < bytes)
+ iomap_write_failed(iter->inode, pos + written,
+ bytes - written);
+ if (unlikely(copied != written))
+ iov_iter_revert(i, copied - written);
cond_resched();
- if (unlikely(status == 0)) {
+ if (unlikely(written == 0)) {
/*
* A short copy made iomap_write_end() reject the
* thing entirely. Might be memory poisoning
@@ -945,17 +946,17 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
goto retry;
}
} else {
- pos += status;
- written += status;
- length -= status;
+ pos += written;
+ total_written += written;
+ length -= written;
}
} while (iov_iter_count(i) && length);
if (status == -EAGAIN) {
- iov_iter_revert(i, written);
+ iov_iter_revert(i, total_written);
return -EAGAIN;
}
- return written ? written : status;
+ return total_written ? total_written : status;
}
ssize_t
--
2.39.2
From: Zhang Yi <[email protected]>
Since xfs_bmapi_convert_delalloc() only attempts to allocate the entire
delalloc extent and require multiple invocations to allocate the target
offset. So xfs_convert_blocks() add a loop to do this job and we call it
in the write back path, but xfs_convert_blocks() isn't a common helper.
Let's do it in xfs_bmapi_convert_delalloc() and drop
xfs_convert_blocks(), preparing for the post EOF delalloc blocks
converting in the buffered write begin path.
Signed-off-by: Zhang Yi <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
fs/xfs/libxfs/xfs_bmap.c | 34 +++++++++++++++++++++++--
fs/xfs/xfs_aops.c | 54 +++++++++++-----------------------------
2 files changed, 46 insertions(+), 42 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 07dc35de8ce5..e44b9bbe0c4a 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4516,8 +4516,8 @@ xfs_bmapi_write(
* invocations to allocate the target offset if a large enough physical extent
* is not available.
*/
-int
-xfs_bmapi_convert_delalloc(
+static int
+xfs_bmapi_convert_one_delalloc(
struct xfs_inode *ip,
int whichfork,
xfs_off_t offset,
@@ -4648,6 +4648,36 @@ xfs_bmapi_convert_delalloc(
return error;
}
+/*
+ * Pass in a dellalloc extent and convert it to real extents, return the real
+ * extent that maps offset_fsb in iomap.
+ */
+int
+xfs_bmapi_convert_delalloc(
+ struct xfs_inode *ip,
+ int whichfork,
+ loff_t offset,
+ struct iomap *iomap,
+ unsigned int *seq)
+{
+ int error;
+
+ /*
+ * Attempt to allocate whatever delalloc extent currently backs offset
+ * and put the result into iomap. Allocate in a loop because it may
+ * take several attempts to allocate real blocks for a contiguous
+ * delalloc extent if free space is sufficiently fragmented.
+ */
+ do {
+ error = xfs_bmapi_convert_one_delalloc(ip, whichfork, offset,
+ iomap, seq);
+ if (error)
+ return error;
+ } while (iomap->offset + iomap->length <= offset);
+
+ return 0;
+}
+
int
xfs_bmapi_remap(
struct xfs_trans *tp,
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 813f85156b0c..6479e0dac69d 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -233,45 +233,6 @@ xfs_imap_valid(
return true;
}
-/*
- * Pass in a dellalloc extent and convert it to real extents, return the real
- * extent that maps offset_fsb in wpc->iomap.
- *
- * The current page is held locked so nothing could have removed the block
- * backing offset_fsb, although it could have moved from the COW to the data
- * fork by another thread.
- */
-static int
-xfs_convert_blocks(
- struct iomap_writepage_ctx *wpc,
- struct xfs_inode *ip,
- int whichfork,
- loff_t offset)
-{
- int error;
- unsigned *seq;
-
- if (whichfork == XFS_COW_FORK)
- seq = &XFS_WPC(wpc)->cow_seq;
- else
- seq = &XFS_WPC(wpc)->data_seq;
-
- /*
- * Attempt to allocate whatever delalloc extent currently backs offset
- * and put the result into wpc->iomap. Allocate in a loop because it
- * may take several attempts to allocate real blocks for a contiguous
- * delalloc extent if free space is sufficiently fragmented.
- */
- do {
- error = xfs_bmapi_convert_delalloc(ip, whichfork, offset,
- &wpc->iomap, seq);
- if (error)
- return error;
- } while (wpc->iomap.offset + wpc->iomap.length <= offset);
-
- return 0;
-}
-
static int
xfs_map_blocks(
struct iomap_writepage_ctx *wpc,
@@ -289,6 +250,7 @@ xfs_map_blocks(
struct xfs_iext_cursor icur;
int retries = 0;
int error = 0;
+ unsigned int *seq;
if (xfs_is_shutdown(mp))
return -EIO;
@@ -386,7 +348,19 @@ xfs_map_blocks(
trace_xfs_map_blocks_found(ip, offset, count, whichfork, &imap);
return 0;
allocate_blocks:
- error = xfs_convert_blocks(wpc, ip, whichfork, offset);
+ /*
+ * Convert a dellalloc extent to a real one. The current page is held
+ * locked so nothing could have removed the block backing offset_fsb,
+ * although it could have moved from the COW to the data fork by another
+ * thread.
+ */
+ if (whichfork == XFS_COW_FORK)
+ seq = &XFS_WPC(wpc)->cow_seq;
+ else
+ seq = &XFS_WPC(wpc)->data_seq;
+
+ error = xfs_bmapi_convert_delalloc(ip, whichfork, offset,
+ &wpc->iomap, seq);
if (error) {
/*
* If we failed to find the extent in the COW fork we might have
--
2.39.2
From: Zhang Yi <[email protected]>
Since iomap_write_end() can never return a partial write length, the
comparison between written, copied and bytes becomes useless, just
merge them with the unwritten branch.
Signed-off-by: Zhang Yi <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
fs/iomap/buffered-io.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index dc863a76c72a..a111e8b816df 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -937,11 +937,6 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
if (old_size < pos)
pagecache_isize_extended(iter->inode, old_size, pos);
- if (written < bytes)
- iomap_write_failed(iter->inode, pos + written,
- bytes - written);
- if (unlikely(copied != written))
- iov_iter_revert(i, copied - written);
cond_resched();
if (unlikely(written == 0)) {
@@ -951,6 +946,9 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
* halfway through, might be a race with munmap,
* might be severe memory pressure.
*/
+ iomap_write_failed(iter->inode, pos, bytes);
+ iov_iter_revert(i, copied);
+
if (chunk > PAGE_SIZE)
chunk /= 2;
if (copied) {
--
2.39.2
From: Zhang Yi <[email protected]>
For now, we can make sure iomap_write_end() always return 0 or copied
bytes, so instead of return written bytes, convert to return a boolean
to indicate the copied bytes have been written to the pagecache.
Signed-off-by: Zhang Yi <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
fs/iomap/buffered-io.c | 48 +++++++++++++++++++++++++++---------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 291648c61a32..dc863a76c72a 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -790,7 +790,7 @@ static int iomap_write_begin(struct iomap_iter *iter, loff_t pos,
return status;
}
-static size_t __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
+static bool __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
size_t copied, struct folio *folio)
{
flush_dcache_folio(folio);
@@ -807,14 +807,14 @@ static size_t __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
* redo the whole thing.
*/
if (unlikely(copied < len && !folio_test_uptodate(folio)))
- return 0;
+ return false;
iomap_set_range_uptodate(folio, offset_in_folio(folio, pos), len);
iomap_set_range_dirty(folio, offset_in_folio(folio, pos), copied);
filemap_dirty_folio(inode->i_mapping, folio);
- return copied;
+ return true;
}
-static size_t iomap_write_end_inline(const struct iomap_iter *iter,
+static void iomap_write_end_inline(const struct iomap_iter *iter,
struct folio *folio, loff_t pos, size_t copied)
{
const struct iomap *iomap = &iter->iomap;
@@ -829,20 +829,31 @@ static size_t iomap_write_end_inline(const struct iomap_iter *iter,
kunmap_local(addr);
mark_inode_dirty(iter->inode);
- return copied;
}
-/* Returns the number of bytes copied. May be 0. Cannot be an errno. */
-static size_t iomap_write_end(struct iomap_iter *iter, loff_t pos, size_t len,
+/*
+ * Returns true if all copied bytes have been written to the pagecache,
+ * otherwise return false.
+ */
+static bool iomap_write_end(struct iomap_iter *iter, loff_t pos, size_t len,
size_t copied, struct folio *folio)
{
const struct iomap *srcmap = iomap_iter_srcmap(iter);
- if (srcmap->type == IOMAP_INLINE)
- return iomap_write_end_inline(iter, folio, pos, copied);
- if (srcmap->flags & IOMAP_F_BUFFER_HEAD)
- return block_write_end(NULL, iter->inode->i_mapping, pos, len,
- copied, &folio->page, NULL);
+ if (srcmap->type == IOMAP_INLINE) {
+ iomap_write_end_inline(iter, folio, pos, copied);
+ return true;
+ }
+
+ if (srcmap->flags & IOMAP_F_BUFFER_HEAD) {
+ size_t bh_written;
+
+ bh_written = block_write_end(NULL, iter->inode->i_mapping, pos,
+ len, copied, &folio->page, NULL);
+ WARN_ON_ONCE(bh_written != copied && bh_written != 0);
+ return bh_written == copied;
+ }
+
return __iomap_write_end(iter->inode, pos, len, copied, folio);
}
@@ -907,7 +918,8 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
flush_dcache_folio(folio);
copied = copy_folio_from_iter_atomic(folio, offset, bytes, i);
- written = iomap_write_end(iter, pos, bytes, copied, folio);
+ written = iomap_write_end(iter, pos, bytes, copied, folio) ?
+ copied : 0;
/*
* Update the in-memory inode size after copying the data into
@@ -1285,6 +1297,7 @@ static loff_t iomap_unshare_iter(struct iomap_iter *iter)
int status;
size_t offset;
size_t bytes = min_t(u64, SIZE_MAX, length);
+ bool ret;
status = iomap_write_begin(iter, pos, bytes, &folio);
if (unlikely(status))
@@ -1296,9 +1309,9 @@ static loff_t iomap_unshare_iter(struct iomap_iter *iter)
if (bytes > folio_size(folio) - offset)
bytes = folio_size(folio) - offset;
- bytes = iomap_write_end(iter, pos, bytes, bytes, folio);
+ ret = iomap_write_end(iter, pos, bytes, bytes, folio);
__iomap_put_folio(iter, pos, bytes, folio);
- if (WARN_ON_ONCE(bytes == 0))
+ if (WARN_ON_ONCE(!ret))
return -EIO;
cond_resched();
@@ -1347,6 +1360,7 @@ static loff_t iomap_zero_iter(struct iomap_iter *iter, bool *did_zero)
int status;
size_t offset;
size_t bytes = min_t(u64, SIZE_MAX, length);
+ bool ret;
status = iomap_write_begin(iter, pos, bytes, &folio);
if (status)
@@ -1361,9 +1375,9 @@ static loff_t iomap_zero_iter(struct iomap_iter *iter, bool *did_zero)
folio_zero_range(folio, offset, bytes);
folio_mark_accessed(folio);
- bytes = iomap_write_end(iter, pos, bytes, bytes, folio);
+ ret = iomap_write_end(iter, pos, bytes, bytes, folio);
__iomap_put_folio(iter, pos, bytes, folio);
- if (WARN_ON_ONCE(bytes == 0))
+ if (WARN_ON_ONCE(!ret))
return -EIO;
pos += bytes;
--
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/[email protected]/
> [2] https://lore.kernel.org/linux-ext4/[email protected]/
> [3] https://lore.kernel.org/linux-ext4/[email protected]/
>
> 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/[email protected]/
> > [2] https://lore.kernel.org/linux-ext4/[email protected]/
> > [3] https://lore.kernel.org/linux-ext4/[email protected]/
> >
> > 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/[email protected]/
>> > [2] https://lore.kernel.org/linux-ext4/[email protected]/
>> > [3] https://lore.kernel.org/linux-ext4/[email protected]/
>> >
>> > 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