All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
To: jaegeuk@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: [f2fs-dev] [PATCH] f2fs: fix to tag STATX_DIOALIGN only if inode support dio
Date: Tue, 10 Sep 2024 20:57:53 +0800	[thread overview]
Message-ID: <20240910125753.80502-1-chao@kernel.org> (raw)

After commit 5c8764f8679e ("f2fs: fix to force buffered IO on inline_data
inode"), f2fs starts to force using buffered IO on inline_data inode.

And also, it will cause f2fs_getattr() returning invalid zeroed value on
.dio_mem_align and .dio_offset_align fields, however, STATX_DIOALIGN flag
was been tagged. User may use zeroed .stx_dio_offset_align value
since STATX_DIOALIGN was been tagged, then it causes a deadloop during
generic/465 test due to below logic:

align=stx_dio_offset_align(it equals to zero)
page_size=4096
while [ $align -le $page_size ]; do
	echo "$AIO_TEST -a $align -d $testfile.$align" >> $seqres.full
	$AIO_TEST -a $align -d $testfile.$align 2>&1 | tee -a $seqres.full
	align=$((align * 2))
done

Quoted from description of statx manual:

" If  a  filesystem  does  not support a field or if it has an
  unrepresentable value (for instance, a file with an exotic type),
  then the mask bit corresponding to that field will be cleared in
  stx_mask even if the user asked for it and a dummy value will be
  filled in for compatibility purposes if one is available (e.g.,
  a dummy UID and GID may be specified to mount under some
  circumstances)."

We should not set STATX_DIOALIGN flag in kstat.stx_mask if inode
does not support DIO, so that it can indicate related fields contain
dummy value, and avoid following incorrect use of them.

Fixes: c8c02272a9f7 ("f2fs: support STATX_DIOALIGN")
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/file.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 99903eafa7fe..f0b8b77e93ba 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -906,14 +906,11 @@ int f2fs_getattr(struct mnt_idmap *idmap, const struct path *path,
 	 * f2fs sometimes supports DIO reads but not DIO writes.  STATX_DIOALIGN
 	 * cannot represent that, so in that case we report no DIO support.
 	 */
-	if ((request_mask & STATX_DIOALIGN) && S_ISREG(inode->i_mode)) {
-		unsigned int bsize = i_blocksize(inode);
-
+	if ((request_mask & STATX_DIOALIGN) && S_ISREG(inode->i_mode) &&
+				!f2fs_force_buffered_io(inode, WRITE)) {
+		stat->dio_mem_align = F2FS_BLKSIZE;
+		stat->dio_offset_align = F2FS_BLKSIZE;
 		stat->result_mask |= STATX_DIOALIGN;
-		if (!f2fs_force_buffered_io(inode, WRITE)) {
-			stat->dio_mem_align = bsize;
-			stat->dio_offset_align = bsize;
-		}
 	}
 
 	flags = fi->i_flags;
-- 
2.40.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <chao@kernel.org>
To: jaegeuk@kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, Chao Yu <chao@kernel.org>
Subject: [PATCH] f2fs: fix to tag STATX_DIOALIGN only if inode support dio
Date: Tue, 10 Sep 2024 20:57:53 +0800	[thread overview]
Message-ID: <20240910125753.80502-1-chao@kernel.org> (raw)

After commit 5c8764f8679e ("f2fs: fix to force buffered IO on inline_data
inode"), f2fs starts to force using buffered IO on inline_data inode.

And also, it will cause f2fs_getattr() returning invalid zeroed value on
.dio_mem_align and .dio_offset_align fields, however, STATX_DIOALIGN flag
was been tagged. User may use zeroed .stx_dio_offset_align value
since STATX_DIOALIGN was been tagged, then it causes a deadloop during
generic/465 test due to below logic:

align=stx_dio_offset_align(it equals to zero)
page_size=4096
while [ $align -le $page_size ]; do
	echo "$AIO_TEST -a $align -d $testfile.$align" >> $seqres.full
	$AIO_TEST -a $align -d $testfile.$align 2>&1 | tee -a $seqres.full
	align=$((align * 2))
done

Quoted from description of statx manual:

" If  a  filesystem  does  not support a field or if it has an
  unrepresentable value (for instance, a file with an exotic type),
  then the mask bit corresponding to that field will be cleared in
  stx_mask even if the user asked for it and a dummy value will be
  filled in for compatibility purposes if one is available (e.g.,
  a dummy UID and GID may be specified to mount under some
  circumstances)."

We should not set STATX_DIOALIGN flag in kstat.stx_mask if inode
does not support DIO, so that it can indicate related fields contain
dummy value, and avoid following incorrect use of them.

Fixes: c8c02272a9f7 ("f2fs: support STATX_DIOALIGN")
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/file.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 99903eafa7fe..f0b8b77e93ba 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -906,14 +906,11 @@ int f2fs_getattr(struct mnt_idmap *idmap, const struct path *path,
 	 * f2fs sometimes supports DIO reads but not DIO writes.  STATX_DIOALIGN
 	 * cannot represent that, so in that case we report no DIO support.
 	 */
-	if ((request_mask & STATX_DIOALIGN) && S_ISREG(inode->i_mode)) {
-		unsigned int bsize = i_blocksize(inode);
-
+	if ((request_mask & STATX_DIOALIGN) && S_ISREG(inode->i_mode) &&
+				!f2fs_force_buffered_io(inode, WRITE)) {
+		stat->dio_mem_align = F2FS_BLKSIZE;
+		stat->dio_offset_align = F2FS_BLKSIZE;
 		stat->result_mask |= STATX_DIOALIGN;
-		if (!f2fs_force_buffered_io(inode, WRITE)) {
-			stat->dio_mem_align = bsize;
-			stat->dio_offset_align = bsize;
-		}
 	}
 
 	flags = fi->i_flags;
-- 
2.40.1


             reply	other threads:[~2024-09-10 12:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-10 12:57 Chao Yu via Linux-f2fs-devel [this message]
2024-09-10 12:57 ` [PATCH] f2fs: fix to tag STATX_DIOALIGN only if inode support dio Chao Yu
2024-09-10 17:06 ` [f2fs-dev] " Eric Biggers via Linux-f2fs-devel
2024-09-10 17:06   ` Eric Biggers
2024-09-11  3:06   ` Chao Yu via Linux-f2fs-devel
2024-09-11  3:06     ` Chao Yu
2024-09-11  3:13     ` Eric Biggers via Linux-f2fs-devel
2024-09-11  3:13       ` Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240910125753.80502-1-chao@kernel.org \
    --to=linux-f2fs-devel@lists.sourceforge.net \
    --cc=chao@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.