When an O_DIRECT write falls back to buffered I/O,
f2fs_flush_buffered_write() uses orig_pos to flush and invalidate
the page cache.

For O_APPEND writes, f2fs_write_checks() updates iocb->ki_pos to
the end of the file after orig_pos has been saved. As a result,
the cleanup range can differ from the actual buffered write range.

Save the write position after f2fs_write_checks() and use it for
the forced buffered I/O cleanup.

Fixes: 92318f20d703 ("f2fs: preserve direct write semantics when buffering is forced")
Signed-off-by: Seongjae Jeong <jsjlee1020@gmail.com>
---
 fs/f2fs/file.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b99d9cdf9ba7..bf61545603e5 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -5313,6 +5313,7 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	const loff_t pos = iocb->ki_pos;
 	const ssize_t count = iov_iter_count(from);
 	ssize_t ret;
+	loff_t bufio_start_pos;
 
 	if (unlikely(f2fs_cp_error(F2FS_I_SB(inode)))) {
 		ret = -EIO;
@@ -5343,6 +5344,8 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (ret <= 0)
 		goto out_unlock;
 
+	bufio_start_pos = iocb->ki_pos;
+
 	/* Determine whether we will do a direct write or a buffered write. */
 	dio = f2fs_should_use_dio(inode, iocb, from);
 
@@ -5396,8 +5399,8 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	 */
 	if (ret > 0 && !dio && (iocb->ki_flags & IOCB_DIRECT))
 		f2fs_flush_buffered_write(iocb->ki_filp->f_mapping,
-					  orig_pos,
-					  orig_pos + ret - 1);
+					  bufio_start_pos,
+					  bufio_start_pos + ret - 1);
 
 	return ret;
 }
-- 
2.53.0

On 8/14/26 18:52, Seongjae Jeong wrote:
> When an O_DIRECT write falls back to buffered I/O,
> f2fs_flush_buffered_write() uses orig_pos to flush and invalidate
> the page cache.
> 
> For O_APPEND writes, f2fs_write_checks() updates iocb->ki_pos to
> the end of the file after orig_pos has been saved. As a result,
> the cleanup range can differ from the actual buffered write range.

If generic_write_checks() can adjust writing position or amount of bytes
to write, shouldn't we access iocb->ki_pos and iov_iter_count(from) after
generic_write_checks()?

Thanks,

> 
> Save the write position after f2fs_write_checks() and use it for
> the forced buffered I/O cleanup.
> 
> Fixes: 92318f20d703 ("f2fs: preserve direct write semantics when buffering is forced")
> Signed-off-by: Seongjae Jeong <jsjlee1020@gmail.com>
> ---
>   fs/f2fs/file.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index b99d9cdf9ba7..bf61545603e5 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -5313,6 +5313,7 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   	const loff_t pos = iocb->ki_pos;
>   	const ssize_t count = iov_iter_count(from);
>   	ssize_t ret;
> +	loff_t bufio_start_pos;
>   
>   	if (unlikely(f2fs_cp_error(F2FS_I_SB(inode)))) {
>   		ret = -EIO;
> @@ -5343,6 +5344,8 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   	if (ret <= 0)
>   		goto out_unlock;
>   
> +	bufio_start_pos = iocb->ki_pos;
> +
>   	/* Determine whether we will do a direct write or a buffered write. */
>   	dio = f2fs_should_use_dio(inode, iocb, from);
>   
> @@ -5396,8 +5399,8 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   	 */
>   	if (ret > 0 && !dio && (iocb->ki_flags & IOCB_DIRECT))
>   		f2fs_flush_buffered_write(iocb->ki_filp->f_mapping,
> -					  orig_pos,
> -					  orig_pos + ret - 1);
> +					  bufio_start_pos,
> +					  bufio_start_pos + ret - 1);
>   
>   	return ret;
>   }

> If generic_write_checks() can adjust writing position or amount of bytes
> to write, shouldn't we access iocb->ki_pos and iov_iter_count(from) after
> generic_write_checks()?

Yes, agreed. generic_write_checks() can adjust both iocb->ki_pos and
the iterator count.

I noticed that the pinned file overwrite check uses pos and count
saved before f2fs_write_checks(). I plan to keep the check in
f2fs_file_write_iter(), but move it after f2fs_write_checks() and use
the adjusted iocb->ki_pos and iov_iter_count(from).

This also keeps the change local to f2fs_file_write_iter(), where other
checks are already performed after f2fs_write_checks().

For the buffered cleanup, as in my original patch, I plan to save the
adjusted write position after f2fs_write_checks() and use it for
f2fs_flush_buffered_write().

Does this approach look reasonable to you?

Thanks,
Seongjae

On 8/18/26 20:23, 정성재 wrote:
>> If generic_write_checks() can adjust writing position or amount of bytes
>> to write, shouldn't we access iocb->ki_pos and iov_iter_count(from) after
>> generic_write_checks()?
> 
> Yes, agreed. generic_write_checks() can adjust both iocb->ki_pos and
> the iterator count.
> 
> I noticed that the pinned file overwrite check uses pos and count
> saved before f2fs_write_checks(). I plan to keep the check in
> f2fs_file_write_iter(), but move it after f2fs_write_checks() and use
> the adjusted iocb->ki_pos and iov_iter_count(from).

Yeah, I think it's the correct way.

> 
> This also keeps the change local to f2fs_file_write_iter(), where other
> checks are already performed after f2fs_write_checks().
> 
> For the buffered cleanup, as in my original patch, I plan to save the
> adjusted write position after f2fs_write_checks() and use it for
> f2fs_flush_buffered_write().
> 
> Does this approach look reasonable to you?

It make sense, please go ahead. :)

Thanks,

> 
> Thanks,
> Seongjae