mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/4] iomap: allow partial folio write with iomap_folio_state
@ 2025-08-10  4:48 alexjlzheng
  2025-08-10  4:48 ` [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size alexjlzheng
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: alexjlzheng @ 2025-08-10  4:48 UTC (permalink / raw)
  To: brauner, djwong; +Cc: linux-xfs, linux-fsdevel, linux-kernel, Jinliang Zheng

From: Jinliang Zheng <alexjlzheng@tencent.com>

With iomap_folio_state, we can identify uptodate states at the block
level, and a read_folio reading can correctly handle partially
uptodate folios.

Therefore, when a partial write occurs, accept the block-aligned
partial write instead of rejecting the entire write.

This patchset has been tested by xfstests' generic and xfs group, and
there's no new failed cases compared to the lastest upstream version kernel.

Jinliang Zheng (4):
  iomap: make sure iomap_adjust_read_range() are aligned with block_size
  iomap: move iter revert case out of the unwritten branch
  iomap: make iomap_write_end() return the number of written length again
  iomap: don't abandon the whole thing with iomap_folio_state

 fs/iomap/buffered-io.c | 68 +++++++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 21 deletions(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size
  2025-08-10  4:48 [PATCH 0/4] iomap: allow partial folio write with iomap_folio_state alexjlzheng
@ 2025-08-10  4:48 ` alexjlzheng
  2025-08-10  7:20   ` kernel test robot
  2025-08-10  4:48 ` [PATCH 2/4] iomap: move iter revert case out of the unwritten branch alexjlzheng
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: alexjlzheng @ 2025-08-10  4:48 UTC (permalink / raw)
  To: brauner, djwong; +Cc: linux-xfs, linux-fsdevel, linux-kernel, Jinliang Zheng

From: Jinliang Zheng <alexjlzheng@tencent.com>

iomap_folio_state marks the uptodate state in units of block_size, so
it is better to check that pos and length are aligned with block_size.

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 fs/iomap/buffered-io.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index fd827398afd2..27fa93ca8675 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -234,6 +234,9 @@ static void iomap_adjust_read_range(struct inode *inode, struct folio *folio,
 	unsigned first = poff >> block_bits;
 	unsigned last = (poff + plen - 1) >> block_bits;
 
+	BUG_ON(*pos % block_size);
+	BUG_ON(length % block_size);
+
 	/*
 	 * If the block size is smaller than the page size, we need to check the
 	 * per-block uptodate status and adjust the offset and length if needed
-- 
2.49.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/4] iomap: move iter revert case out of the unwritten branch
  2025-08-10  4:48 [PATCH 0/4] iomap: allow partial folio write with iomap_folio_state alexjlzheng
  2025-08-10  4:48 ` [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size alexjlzheng
@ 2025-08-10  4:48 ` alexjlzheng
  2025-08-10  4:48 ` [PATCH 3/4] iomap: make iomap_write_end() return the number of written length again alexjlzheng
  2025-08-10  4:48 ` [PATCH 4/4] iomap: don't abandon the whole thing with iomap_folio_state alexjlzheng
  3 siblings, 0 replies; 12+ messages in thread
From: alexjlzheng @ 2025-08-10  4:48 UTC (permalink / raw)
  To: brauner, djwong; +Cc: linux-xfs, linux-fsdevel, linux-kernel, Jinliang Zheng

From: Jinliang Zheng <alexjlzheng@tencent.com>

This reverts commit e1f453d4336d ("iomap: do some small logical
cleanup in buffered write"), for preparetion for the next patches
which allow iomap_write_end() return a partial write length.

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 fs/iomap/buffered-io.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 27fa93ca8675..df801220f4b3 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -1019,6 +1019,11 @@ static int 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)) {
@@ -1028,9 +1033,6 @@ static int 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.49.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/4] iomap: make iomap_write_end() return the number of written length again
  2025-08-10  4:48 [PATCH 0/4] iomap: allow partial folio write with iomap_folio_state alexjlzheng
  2025-08-10  4:48 ` [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size alexjlzheng
  2025-08-10  4:48 ` [PATCH 2/4] iomap: move iter revert case out of the unwritten branch alexjlzheng
@ 2025-08-10  4:48 ` alexjlzheng
  2025-08-10  4:48 ` [PATCH 4/4] iomap: don't abandon the whole thing with iomap_folio_state alexjlzheng
  3 siblings, 0 replies; 12+ messages in thread
From: alexjlzheng @ 2025-08-10  4:48 UTC (permalink / raw)
  To: brauner, djwong; +Cc: linux-xfs, linux-fsdevel, linux-kernel, Jinliang Zheng

From: Jinliang Zheng <alexjlzheng@tencent.com>

In the next patch, we allow iomap_write_end() to conditionally accept
partial writes, so this patch makes iomap_write_end() return the number
of accepted write bytes in preparation for the next patch.

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 fs/iomap/buffered-io.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index df801220f4b3..1b92a0f15bc1 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -873,7 +873,7 @@ static int iomap_write_begin(struct iomap_iter *iter,
 	return status;
 }
 
-static bool __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
+static int __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
 		size_t copied, struct folio *folio)
 {
 	flush_dcache_folio(folio);
@@ -890,11 +890,11 @@ static bool __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 false;
+		return 0;
 	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 true;
+	return copied;
 }
 
 static void iomap_write_end_inline(const struct iomap_iter *iter,
@@ -915,10 +915,10 @@ static void iomap_write_end_inline(const struct iomap_iter *iter,
 }
 
 /*
- * Returns true if all copied bytes have been written to the pagecache,
- * otherwise return false.
+ * Returns number of copied bytes have been written to the pagecache,
+ * zero if block is partial update.
  */
-static bool iomap_write_end(struct iomap_iter *iter, size_t len, size_t copied,
+static int iomap_write_end(struct iomap_iter *iter, size_t len, size_t copied,
 		struct folio *folio)
 {
 	const struct iomap *srcmap = iomap_iter_srcmap(iter);
@@ -926,7 +926,7 @@ static bool iomap_write_end(struct iomap_iter *iter, size_t len, size_t copied,
 
 	if (srcmap->type == IOMAP_INLINE) {
 		iomap_write_end_inline(iter, folio, pos, copied);
-		return true;
+		return copied;
 	}
 
 	if (srcmap->flags & IOMAP_F_BUFFER_HEAD) {
@@ -934,7 +934,7 @@ static bool iomap_write_end(struct iomap_iter *iter, size_t len, size_t copied,
 
 		bh_written = block_write_end(pos, len, copied, folio);
 		WARN_ON_ONCE(bh_written != copied && bh_written != 0);
-		return bh_written == copied;
+		return bh_written;
 	}
 
 	return __iomap_write_end(iter->inode, pos, len, copied, folio);
@@ -1000,8 +1000,7 @@ static int 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, bytes, copied, folio) ?
-			  copied : 0;
+		written = iomap_write_end(iter, bytes, copied, folio);
 
 		/*
 		 * Update the in-memory inode size after copying the data into
@@ -1315,7 +1314,7 @@ static int iomap_unshare_iter(struct iomap_iter *iter,
 	do {
 		struct folio *folio;
 		size_t offset;
-		bool ret;
+		int ret;
 
 		bytes = min_t(u64, SIZE_MAX, bytes);
 		status = iomap_write_begin(iter, write_ops, &folio, &offset,
@@ -1327,7 +1326,7 @@ static int iomap_unshare_iter(struct iomap_iter *iter,
 
 		ret = iomap_write_end(iter, bytes, bytes, folio);
 		__iomap_put_folio(iter, write_ops, bytes, folio);
-		if (WARN_ON_ONCE(!ret))
+		if (WARN_ON_ONCE(ret != bytes))
 			return -EIO;
 
 		cond_resched();
@@ -1388,7 +1387,7 @@ static int iomap_zero_iter(struct iomap_iter *iter, bool *did_zero,
 	do {
 		struct folio *folio;
 		size_t offset;
-		bool ret;
+		int ret;
 
 		bytes = min_t(u64, SIZE_MAX, bytes);
 		status = iomap_write_begin(iter, write_ops, &folio, &offset,
@@ -1406,7 +1405,7 @@ static int iomap_zero_iter(struct iomap_iter *iter, bool *did_zero,
 
 		ret = iomap_write_end(iter, bytes, bytes, folio);
 		__iomap_put_folio(iter, write_ops, bytes, folio);
-		if (WARN_ON_ONCE(!ret))
+		if (WARN_ON_ONCE(ret != bytes))
 			return -EIO;
 
 		status = iomap_iter_advance(iter, &bytes);
-- 
2.49.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/4] iomap: don't abandon the whole thing with iomap_folio_state
  2025-08-10  4:48 [PATCH 0/4] iomap: allow partial folio write with iomap_folio_state alexjlzheng
                   ` (2 preceding siblings ...)
  2025-08-10  4:48 ` [PATCH 3/4] iomap: make iomap_write_end() return the number of written length again alexjlzheng
@ 2025-08-10  4:48 ` alexjlzheng
  2025-08-10  7:33   ` kernel test robot
  3 siblings, 1 reply; 12+ messages in thread
From: alexjlzheng @ 2025-08-10  4:48 UTC (permalink / raw)
  To: brauner, djwong; +Cc: linux-xfs, linux-fsdevel, linux-kernel, Jinliang Zheng

From: Jinliang Zheng <alexjlzheng@tencent.com>

With iomap_folio_state, we can identify uptodate states at the block
level, and a read_folio reading can correctly handle partially
uptodate folios.

Therefore, when a partial write occurs, accept the block-aligned
partial write instead of rejecting the entire write.

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 fs/iomap/buffered-io.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 1b92a0f15bc1..10701923d968 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -873,6 +873,25 @@ static int iomap_write_begin(struct iomap_iter *iter,
 	return status;
 }
 
+static int iomap_trim_tail_partial(struct inode *inode, loff_t pos,
+		size_t copied, struct folio *folio)
+{
+	struct iomap_folio_state *ifs = folio->private;
+	unsigned block_size, last_blk, last_blk_bytes;
+
+	if (!ifs || !copied)
+		return 0;
+
+	block_size = 1 << inode->i_blkbits;
+	last_blk = offset_in_folio(folio, pos + copied - 1) >> inode->i_blkbits;
+	last_blk_bytes = (pos + copied) % block_size;
+
+	if (!ifs_block_is_uptodate(ifs, last_blk))
+		copied -= min(copied, last_blk_bytes);
+
+	return copied;
+}
+
 static int __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
 		size_t copied, struct folio *folio)
 {
@@ -886,12 +905,15 @@ static int __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
 	 * read_folio might come in and destroy our partial write.
 	 *
 	 * Do the simplest thing and just treat any short write to a
-	 * non-uptodate page as a zero-length write, and force the caller to
-	 * redo the whole thing.
+	 * non-uptodate block as a zero-length write, and force the caller to
+	 * redo the things begin from the block.
 	 */
-	if (unlikely(copied < len && !folio_test_uptodate(folio)))
-		return 0;
-	iomap_set_range_uptodate(folio, offset_in_folio(folio, pos), len);
+	if (unlikely(copied < len && !folio_test_uptodate(folio))) {
+		copied = iomap_trim_tail_partial(inode, pos, copied, folio);
+		if (!copied)
+			return 0;
+	}
+	iomap_set_range_uptodate(folio, offset_in_folio(folio, pos), copied);
 	iomap_set_range_dirty(folio, offset_in_folio(folio, pos), copied);
 	filemap_dirty_folio(inode->i_mapping, folio);
 	return copied;
-- 
2.49.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size
  2025-08-10  4:48 ` [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size alexjlzheng
@ 2025-08-10  7:20   ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-08-10  7:20 UTC (permalink / raw)
  To: alexjlzheng, brauner, djwong
  Cc: oe-kbuild-all, linux-xfs, linux-fsdevel, linux-kernel, Jinliang Zheng

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on linus/master v6.16 next-20250808]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/alexjlzheng-gmail-com/iomap-make-sure-iomap_adjust_read_range-are-aligned-with-block_size/20250810-125014
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20250810044806.3433783-2-alexjlzheng%40tencent.com
patch subject: [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size
config: m68k-allnoconfig (https://download.01.org/0day-ci/archive/20250810/202508101424.M8eWrUjI-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250810/202508101424.M8eWrUjI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508101424.M8eWrUjI-lkp@intel.com/

All errors (new ones prefixed by >>):

   m68k-linux-ld: fs/iomap/buffered-io.o: in function `iomap_adjust_read_range':
>> buffered-io.c:(.text+0xa8a): undefined reference to `__moddi3'
>> m68k-linux-ld: buffered-io.c:(.text+0xaa8): undefined reference to `__moddi3'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] iomap: don't abandon the whole thing with iomap_folio_state
  2025-08-10  4:48 ` [PATCH 4/4] iomap: don't abandon the whole thing with iomap_folio_state alexjlzheng
@ 2025-08-10  7:33   ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-08-10  7:33 UTC (permalink / raw)
  To: alexjlzheng, brauner, djwong
  Cc: oe-kbuild-all, linux-xfs, linux-fsdevel, linux-kernel, Jinliang Zheng

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on linus/master next-20250808]
[cannot apply to v6.16]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/alexjlzheng-gmail-com/iomap-make-sure-iomap_adjust_read_range-are-aligned-with-block_size/20250810-125014
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20250810044806.3433783-5-alexjlzheng%40tencent.com
patch subject: [PATCH 4/4] iomap: don't abandon the whole thing with iomap_folio_state
config: xtensa-allnoconfig (https://download.01.org/0day-ci/archive/20250810/202508101530.0ER4QBD7-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250810/202508101530.0ER4QBD7-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508101530.0ER4QBD7-lkp@intel.com/

All errors (new ones prefixed by >>):

   xtensa-linux-ld: fs/iomap/buffered-io.o: in function `zero_user_segments':
   buffered-io.c:(.text+0x538): undefined reference to `__moddi3'
>> xtensa-linux-ld: buffered-io.c:(.text+0x5c2): undefined reference to `__moddi3'
   xtensa-linux-ld: buffered-io.c:(.text+0x607): undefined reference to `__moddi3'
   xtensa-linux-ld: fs/iomap/buffered-io.o: in function `iomap_read_inline_data':
   buffered-io.c:(.text+0xfa7): undefined reference to `__moddi3'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size
  2025-09-15  8:54       ` Pankaj Raghav (Samsung)
@ 2025-09-15  9:12         ` Jinliang Zheng
  0 siblings, 0 replies; 12+ messages in thread
From: Jinliang Zheng @ 2025-09-15  9:12 UTC (permalink / raw)
  To: kernel
  Cc: alexjlzheng, alexjlzheng, brauner, djwong, hch, linux-fsdevel,
	linux-kernel, linux-xfs, yi.zhang

On Mon, 15 Sep 2025 10:54:00 +0200, kernel@pankajraghav.com wrote:
> On Sun, Sep 14, 2025 at 08:40:06PM +0800, Jinliang Zheng wrote:
> > On Sun, 14 Sep 2025 13:45:16 +0200, kernel@pankajraghav.com wrote:
> > > On Sat, Sep 14, 2025 at 11:37:15AM +0800, alexjlzheng@gmail.com wrote:
> > > > From: Jinliang Zheng <alexjlzheng@tencent.com>
> > > > 
> > > > iomap_folio_state marks the uptodate state in units of block_size, so
> > > > it is better to check that pos and length are aligned with block_size.
> > > > 
> > > > Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
> > > > ---
> > > >  fs/iomap/buffered-io.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > > > index fd827398afd2..0c38333933c6 100644
> > > > --- a/fs/iomap/buffered-io.c
> > > > +++ b/fs/iomap/buffered-io.c
> > > > @@ -234,6 +234,9 @@ static void iomap_adjust_read_range(struct inode *inode, struct folio *folio,
> > > >  	unsigned first = poff >> block_bits;
> > > >  	unsigned last = (poff + plen - 1) >> block_bits;
> > > >  
> > > > +	WARN_ON(*pos & (block_size - 1));
> > > > +	WARN_ON(length & (block_size - 1));
> > > Any reason you chose WARN_ON instead of WARN_ON_ONCE?
> > 
> > I just think it's a fatal error that deserves attention every time
> > it's triggered.
> > 
> 
> Is this a general change or does your later changes depend on these on
> warning to work correctly?

No, there is no functional change.

I added it only because the correctness of iomap_adjust_read_range() depends on
it, so it's better to hightlight it now.

```
	/* move forward for each leading block marked uptodate */
	for (i = first; i <= last; i++) {
		if (!ifs_block_is_uptodate(ifs, i))
			break;
		*pos += block_size; <-------------------- if not aligned, ...
		poff += block_size;
		plen -= block_size;
		first++;
	}
```

> 
> > > 
> > > I don't see WARN_ON being used in iomap/buffered-io.c.
> > 
> > I'm not sure if there are any community guidelines for using these
> > two macros. If there are, please let me know and I'll be happy to
> > follow them as a guide.
> 
> We typically use WARN_ON_ONCE to prevent spamming.

If you think it's better, I will send a new version.

thanks,
Jinliang Zheng. :)

> 
> --
> Pankaj

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size
  2025-09-14 12:40     ` Jinliang Zheng
@ 2025-09-15  8:54       ` Pankaj Raghav (Samsung)
  2025-09-15  9:12         ` Jinliang Zheng
  0 siblings, 1 reply; 12+ messages in thread
From: Pankaj Raghav (Samsung) @ 2025-09-15  8:54 UTC (permalink / raw)
  To: Jinliang Zheng
  Cc: alexjlzheng, brauner, djwong, hch, linux-fsdevel, linux-kernel,
	linux-xfs, yi.zhang

On Sun, Sep 14, 2025 at 08:40:06PM +0800, Jinliang Zheng wrote:
> On Sun, 14 Sep 2025 13:45:16 +0200, kernel@pankajraghav.com wrote:
> > On Sat, Sep 14, 2025 at 11:37:15AM +0800, alexjlzheng@gmail.com wrote:
> > > From: Jinliang Zheng <alexjlzheng@tencent.com>
> > > 
> > > iomap_folio_state marks the uptodate state in units of block_size, so
> > > it is better to check that pos and length are aligned with block_size.
> > > 
> > > Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
> > > ---
> > >  fs/iomap/buffered-io.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > > index fd827398afd2..0c38333933c6 100644
> > > --- a/fs/iomap/buffered-io.c
> > > +++ b/fs/iomap/buffered-io.c
> > > @@ -234,6 +234,9 @@ static void iomap_adjust_read_range(struct inode *inode, struct folio *folio,
> > >  	unsigned first = poff >> block_bits;
> > >  	unsigned last = (poff + plen - 1) >> block_bits;
> > >  
> > > +	WARN_ON(*pos & (block_size - 1));
> > > +	WARN_ON(length & (block_size - 1));
> > Any reason you chose WARN_ON instead of WARN_ON_ONCE?
> 
> I just think it's a fatal error that deserves attention every time
> it's triggered.
> 

Is this a general change or does your later changes depend on these on
warning to work correctly?

> > 
> > I don't see WARN_ON being used in iomap/buffered-io.c.
> 
> I'm not sure if there are any community guidelines for using these
> two macros. If there are, please let me know and I'll be happy to
> follow them as a guide.

We typically use WARN_ON_ONCE to prevent spamming.

--
Pankaj

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size
  2025-09-14 11:45   ` Pankaj Raghav (Samsung)
@ 2025-09-14 12:40     ` Jinliang Zheng
  2025-09-15  8:54       ` Pankaj Raghav (Samsung)
  0 siblings, 1 reply; 12+ messages in thread
From: Jinliang Zheng @ 2025-09-14 12:40 UTC (permalink / raw)
  To: kernel
  Cc: alexjlzheng, alexjlzheng, brauner, djwong, hch, linux-fsdevel,
	linux-kernel, linux-xfs, yi.zhang

On Sun, 14 Sep 2025 13:45:16 +0200, kernel@pankajraghav.com wrote:
> On Sat, Sep 14, 2025 at 11:37:15AM +0800, alexjlzheng@gmail.com wrote:
> > From: Jinliang Zheng <alexjlzheng@tencent.com>
> > 
> > iomap_folio_state marks the uptodate state in units of block_size, so
> > it is better to check that pos and length are aligned with block_size.
> > 
> > Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
> > ---
> >  fs/iomap/buffered-io.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > index fd827398afd2..0c38333933c6 100644
> > --- a/fs/iomap/buffered-io.c
> > +++ b/fs/iomap/buffered-io.c
> > @@ -234,6 +234,9 @@ static void iomap_adjust_read_range(struct inode *inode, struct folio *folio,
> >  	unsigned first = poff >> block_bits;
> >  	unsigned last = (poff + plen - 1) >> block_bits;
> >  
> > +	WARN_ON(*pos & (block_size - 1));
> > +	WARN_ON(length & (block_size - 1));
> Any reason you chose WARN_ON instead of WARN_ON_ONCE?

I just think it's a fatal error that deserves attention every time
it's triggered.

> 
> I don't see WARN_ON being used in iomap/buffered-io.c.

I'm not sure if there are any community guidelines for using these
two macros. If there are, please let me know and I'll be happy to
follow them as a guide.

thanks,
Jinliang Zheng. :)

> --
> Pankaj

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size
  2025-09-13  3:37 ` [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size alexjlzheng
@ 2025-09-14 11:45   ` Pankaj Raghav (Samsung)
  2025-09-14 12:40     ` Jinliang Zheng
  0 siblings, 1 reply; 12+ messages in thread
From: Pankaj Raghav (Samsung) @ 2025-09-14 11:45 UTC (permalink / raw)
  To: alexjlzheng
  Cc: hch, brauner, djwong, yi.zhang, linux-xfs, linux-fsdevel,
	linux-kernel, Jinliang Zheng

On Sat, Sep 13, 2025 at 11:37:15AM +0800, alexjlzheng@gmail.com wrote:
> From: Jinliang Zheng <alexjlzheng@tencent.com>
> 
> iomap_folio_state marks the uptodate state in units of block_size, so
> it is better to check that pos and length are aligned with block_size.
> 
> Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
> ---
>  fs/iomap/buffered-io.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index fd827398afd2..0c38333933c6 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -234,6 +234,9 @@ static void iomap_adjust_read_range(struct inode *inode, struct folio *folio,
>  	unsigned first = poff >> block_bits;
>  	unsigned last = (poff + plen - 1) >> block_bits;
>  
> +	WARN_ON(*pos & (block_size - 1));
> +	WARN_ON(length & (block_size - 1));
Any reason you chose WARN_ON instead of WARN_ON_ONCE?

I don't see WARN_ON being used in iomap/buffered-io.c.
--
Pankaj

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size
  2025-09-13  3:37 [PATCH v4 0/4] allow partial folio write " alexjlzheng
@ 2025-09-13  3:37 ` alexjlzheng
  2025-09-14 11:45   ` Pankaj Raghav (Samsung)
  0 siblings, 1 reply; 12+ messages in thread
From: alexjlzheng @ 2025-09-13  3:37 UTC (permalink / raw)
  To: hch, brauner
  Cc: djwong, yi.zhang, linux-xfs, linux-fsdevel, linux-kernel, Jinliang Zheng

From: Jinliang Zheng <alexjlzheng@tencent.com>

iomap_folio_state marks the uptodate state in units of block_size, so
it is better to check that pos and length are aligned with block_size.

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 fs/iomap/buffered-io.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index fd827398afd2..0c38333933c6 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -234,6 +234,9 @@ static void iomap_adjust_read_range(struct inode *inode, struct folio *folio,
 	unsigned first = poff >> block_bits;
 	unsigned last = (poff + plen - 1) >> block_bits;
 
+	WARN_ON(*pos & (block_size - 1));
+	WARN_ON(length & (block_size - 1));
+
 	/*
 	 * If the block size is smaller than the page size, we need to check the
 	 * per-block uptodate status and adjust the offset and length if needed
-- 
2.49.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-09-15  9:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-10  4:48 [PATCH 0/4] iomap: allow partial folio write with iomap_folio_state alexjlzheng
2025-08-10  4:48 ` [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size alexjlzheng
2025-08-10  7:20   ` kernel test robot
2025-08-10  4:48 ` [PATCH 2/4] iomap: move iter revert case out of the unwritten branch alexjlzheng
2025-08-10  4:48 ` [PATCH 3/4] iomap: make iomap_write_end() return the number of written length again alexjlzheng
2025-08-10  4:48 ` [PATCH 4/4] iomap: don't abandon the whole thing with iomap_folio_state alexjlzheng
2025-08-10  7:33   ` kernel test robot
2025-09-13  3:37 [PATCH v4 0/4] allow partial folio write " alexjlzheng
2025-09-13  3:37 ` [PATCH 1/4] iomap: make sure iomap_adjust_read_range() are aligned with block_size alexjlzheng
2025-09-14 11:45   ` Pankaj Raghav (Samsung)
2025-09-14 12:40     ` Jinliang Zheng
2025-09-15  8:54       ` Pankaj Raghav (Samsung)
2025-09-15  9:12         ` Jinliang Zheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®