Re: [PATCH v5 1/2] exfat: change to get file size from DataLength

Re: [PATCH v5 1/2] exfat: change to get file size from DataLength

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <PUZPR04MB6316F0640983B00CC55D903F8182A@PUZPR04MB6316.apcprd04.prod.outlook.com>
References: <PUZPR04MB6316F0640983B00CC55D903F8182A@PUZPR04MB6316.apcprd04.prod.outlook.com>
TO: "Yuezhang.Mo@sony.com" <Yuezhang.Mo@sony.com>
TO: "linkinjeon@kernel.org" <linkinjeon@kernel.org>
TO: "sj1557.seo@samsung.com" <sj1557.seo@samsung.com>
CC: "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
CC: "Andy.Wu@sony.com" <Andy.Wu@sony.com>
CC: "Wataru.Aoyama@sony.com" <Wataru.Aoyama@sony.com>
CC: "cpgs@samsung.com" <cpgs@samsung.com>

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.7-rc3 next-20231130]
[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/Yuezhang-Mo-sony-com/exfat-do-not-zero-the-extended-part/20231130-164222
base:   linus/master
patch link:    https://lore.kernel.org/r/PUZPR04MB6316F0640983B00CC55D903F8182A%40PUZPR04MB6316.apcprd04.prod.outlook.com
patch subject: [PATCH v5 1/2] exfat: change to get file size from DataLength
:::::: branch date: 13 hours ago
:::::: commit date: 13 hours ago
config: x86_64-randconfig-r081-20231130 (https://download.01.org/0day-ci/archive/20231201/202312010428.73gtRyvj-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20231201/202312010428.73gtRyvj-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>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202312010428.73gtRyvj-lkp@intel.com/

smatch warnings:
fs/exfat/inode.c:525 exfat_direct_IO() warn: bitwise AND condition is false here

vim +525 fs/exfat/inode.c

5f2aa075070cf5b Namjae Jeon          2020-03-02  485  
5f2aa075070cf5b Namjae Jeon          2020-03-02  486  static ssize_t exfat_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
5f2aa075070cf5b Namjae Jeon          2020-03-02  487  {
5f2aa075070cf5b Namjae Jeon          2020-03-02  488  	struct address_space *mapping = iocb->ki_filp->f_mapping;
5f2aa075070cf5b Namjae Jeon          2020-03-02  489  	struct inode *inode = mapping->host;
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  490  	struct exfat_inode_info *ei = EXFAT_I(inode);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  491  	loff_t pos = iocb->ki_pos;
5f2aa075070cf5b Namjae Jeon          2020-03-02  492  	loff_t size = iocb->ki_pos + iov_iter_count(iter);
5f2aa075070cf5b Namjae Jeon          2020-03-02  493  	int rw = iov_iter_rw(iter);
5f2aa075070cf5b Namjae Jeon          2020-03-02  494  	ssize_t ret;
5f2aa075070cf5b Namjae Jeon          2020-03-02  495  
5f2aa075070cf5b Namjae Jeon          2020-03-02  496  	if (rw == WRITE) {
5f2aa075070cf5b Namjae Jeon          2020-03-02  497  		/*
5f2aa075070cf5b Namjae Jeon          2020-03-02  498  		 * FIXME: blockdev_direct_IO() doesn't use ->write_begin(),
5f2aa075070cf5b Namjae Jeon          2020-03-02  499  		 * so we need to update the ->i_size_aligned to block boundary.
5f2aa075070cf5b Namjae Jeon          2020-03-02  500  		 *
5f2aa075070cf5b Namjae Jeon          2020-03-02  501  		 * But we must fill the remaining area or hole by nul for
5f2aa075070cf5b Namjae Jeon          2020-03-02  502  		 * updating ->i_size_aligned
5f2aa075070cf5b Namjae Jeon          2020-03-02  503  		 *
5f2aa075070cf5b Namjae Jeon          2020-03-02  504  		 * Return 0, and fallback to normal buffered write.
5f2aa075070cf5b Namjae Jeon          2020-03-02  505  		 */
5f2aa075070cf5b Namjae Jeon          2020-03-02  506  		if (EXFAT_I(inode)->i_size_aligned < size)
5f2aa075070cf5b Namjae Jeon          2020-03-02  507  			return 0;
5f2aa075070cf5b Namjae Jeon          2020-03-02  508  	}
5f2aa075070cf5b Namjae Jeon          2020-03-02  509  
5f2aa075070cf5b Namjae Jeon          2020-03-02  510  	/*
5f2aa075070cf5b Namjae Jeon          2020-03-02  511  	 * Need to use the DIO_LOCKING for avoiding the race
5f2aa075070cf5b Namjae Jeon          2020-03-02  512  	 * condition of exfat_get_block() and ->truncate().
5f2aa075070cf5b Namjae Jeon          2020-03-02  513  	 */
5f2aa075070cf5b Namjae Jeon          2020-03-02  514  	ret = blockdev_direct_IO(iocb, inode, iter, exfat_get_block);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  515  	if (ret < 0) {
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  516  		if (rw & WRITE)
5f2aa075070cf5b Namjae Jeon          2020-03-02  517  			exfat_write_failed(mapping, size);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  518  
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  519  		if (ret != -EIOCBQUEUED)
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  520  			return ret;
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  521  	} else
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  522  		size = pos + ret;
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  523  
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  524  	/* zero the unwritten part in the partially written block */
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30 @525  	if ((rw & READ) && pos < ei->valid_size && ei->valid_size < size) {
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  526  		iov_iter_revert(iter, size - ei->valid_size);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  527  		iov_iter_zero(size - ei->valid_size, iter);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  528  	}
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  529  
5f2aa075070cf5b Namjae Jeon          2020-03-02  530  	return ret;
5f2aa075070cf5b Namjae Jeon          2020-03-02  531  }
5f2aa075070cf5b Namjae Jeon          2020-03-02  532  

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

[PATCH v4 0/2] exfat: get file size from DataLength

[Yuezhang.Mo]

From the exFAT specification, the file size should get from 'DataLength'
of Stream Extension Directory Entry, not 'ValidDataLength'.

Without this patch set, 'DataLength' is always same with 'ValidDataLength'
and get file size from 'ValidDataLength'. If the file is created by other
exFAT implementation and 'DataLength' is different from 'ValidDataLength',
this exFAT implementation will not be compatible.

Changes for v4:
  - Rebase for linux-6.7-rc1
  - Use block_write_begin() instead of cont_write_begin() in exfat_write_begin()
  - In exfat_cont_expand(), use ei->i_size_ondisk instead of i_size_read() to
    get the number of clusters of the file.

Changes for v3:
  - Rebase to linux-6.6
  - Move update ->valid_size from exfat_file_write_iter() to exfat_write_end()
  - Use block_write_begin() instead of exfat_write_begin() in exfat_file_zeroed_range()
  - Remove exfat_expand_and_zero()

Changes for v2:
  - Fix race when checking i_size on direct i/o read

Yuezhang Mo (2):
  exfat: change to get file size from DataLength
  exfat: do not zeroed the extended part

 fs/exfat/exfat_fs.h |   2 +
 fs/exfat/file.c     | 197 +++++++++++++++++++++++++++++++++++++++-----
 fs/exfat/inode.c    | 110 +++++++++++++++++++++----
 fs/exfat/namei.c    |   6 ++
 4 files changed, 277 insertions(+), 38 deletions(-)

-- 
2.25.1

[PATCH v5 1/2] exfat: change to get file size from DataLength

[Yuezhang.Mo]

In stream extension directory entry, the ValidDataLength
field describes how far into the data stream user data has
been written, and the DataLength field describes the file
size.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Andy Wu <Andy.Wu@sony.com>
Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
---
 fs/exfat/exfat_fs.h |   2 +
 fs/exfat/file.c     | 122 ++++++++++++++++++++++++++++++++++++++++++-
 fs/exfat/inode.c    | 124 ++++++++++++++++++++++++++++++++++++++------
 fs/exfat/namei.c    |   6 +++
 4 files changed, 235 insertions(+), 19 deletions(-)

diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index a7a2c35d74fb..e3b1f8e022df 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -208,6 +208,7 @@ struct exfat_dir_entry {
 	unsigned char flags;
 	unsigned short attr;
 	loff_t size;
+	loff_t valid_size;
 	unsigned int num_subdirs;
 	struct timespec64 atime;
 	struct timespec64 mtime;
@@ -317,6 +318,7 @@ struct exfat_inode_info {
 	loff_t i_size_aligned;
 	/* on-disk position of directory entry or 0 */
 	loff_t i_pos;
+	loff_t valid_size;
 	/* hash by i_location */
 	struct hlist_node i_hash_fat;
 	/* protect bmap against truncate */
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index bfdfafe00993..154f39a03a69 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -11,6 +11,7 @@
 #include <linux/fsnotify.h>
 #include <linux/security.h>
 #include <linux/msdos_fs.h>
+#include <linux/writeback.h>
 
 #include "exfat_raw.h"
 #include "exfat_fs.h"
@@ -26,6 +27,7 @@ static int exfat_cont_expand(struct inode *inode, loff_t size)
 		return err;
 
 	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
+	EXFAT_I(inode)->valid_size = size;
 	mark_inode_dirty(inode);
 
 	if (!IS_SYNC(inode))
@@ -146,6 +148,9 @@ int __exfat_truncate(struct inode *inode)
 		ei->start_clu = EXFAT_EOF_CLUSTER;
 	}
 
+	if (i_size_read(inode) < ei->valid_size)
+		ei->valid_size = i_size_read(inode);
+
 	if (ei->type == TYPE_FILE)
 		ei->attr |= EXFAT_ATTR_ARCHIVE;
 
@@ -474,15 +479,128 @@ int exfat_file_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
 	return blkdev_issue_flush(inode->i_sb->s_bdev);
 }
 
+static int exfat_file_zeroed_range(struct file *file, loff_t start, loff_t end)
+{
+	int err;
+	struct inode *inode = file_inode(file);
+	struct exfat_inode_info *ei = EXFAT_I(inode);
+	struct address_space *mapping = inode->i_mapping;
+	const struct address_space_operations *ops = mapping->a_ops;
+
+	while (start < end) {
+		u32 zerofrom, len;
+		struct page *page = NULL;
+
+		zerofrom = start & (PAGE_SIZE - 1);
+		len = PAGE_SIZE - zerofrom;
+		if (start + len > end)
+			len = end - start;
+
+		err = ops->write_begin(file, mapping, start, len, &page, NULL);
+		if (err)
+			goto out;
+
+		zero_user_segment(page, zerofrom, zerofrom + len);
+
+		err = ops->write_end(file, mapping, start, len, len, page, NULL);
+		if (err < 0)
+			goto out;
+		start += len;
+
+		balance_dirty_pages_ratelimited(mapping);
+		cond_resched();
+	}
+
+	ei->valid_size = end;
+	mark_inode_dirty(inode);
+
+out:
+	return err;
+}
+
+static ssize_t exfat_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+	ssize_t ret;
+	struct file *file = iocb->ki_filp;
+	struct inode *inode = file_inode(file);
+	struct exfat_inode_info *ei = EXFAT_I(inode);
+	loff_t pos = iocb->ki_pos;
+	loff_t valid_size;
+
+	inode_lock(inode);
+
+	valid_size = ei->valid_size;
+
+	ret = generic_write_checks(iocb, iter);
+	if (ret < 0)
+		goto unlock;
+
+	if (pos > valid_size) {
+		ret = exfat_file_zeroed_range(file, valid_size, pos);
+		if (ret < 0 && ret != -ENOSPC) {
+			exfat_err(inode->i_sb,
+				"write: fail to zero from %llu to %llu(%ld)",
+				valid_size, pos, ret);
+		}
+		if (ret < 0)
+			goto unlock;
+	}
+
+	ret = __generic_file_write_iter(iocb, iter);
+	if (ret < 0)
+		goto unlock;
+
+	inode_unlock(inode);
+
+	if (pos > valid_size)
+		pos = valid_size;
+
+	if (iocb_is_dsync(iocb) && iocb->ki_pos > pos) {
+		ssize_t err = vfs_fsync_range(file, pos, iocb->ki_pos - 1,
+				iocb->ki_flags & IOCB_SYNC);
+		if (err < 0)
+			return err;
+	}
+
+	return ret;
+
+unlock:
+	inode_unlock(inode);
+
+	return ret;
+}
+
+static int exfat_file_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	int ret;
+	struct inode *inode = file_inode(file);
+	struct exfat_inode_info *ei = EXFAT_I(inode);
+	loff_t start = ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+	loff_t end = min_t(loff_t, i_size_read(inode),
+			start + vma->vm_end - vma->vm_start);
+
+	if ((vma->vm_flags & VM_WRITE) && ei->valid_size < end) {
+		ret = exfat_file_zeroed_range(file, ei->valid_size, end);
+		if (ret < 0) {
+			exfat_err(inode->i_sb,
+				  "mmap: fail to zero from %llu to %llu(%d)",
+				  start, end, ret);
+			return ret;
+		}
+	}
+
+	return generic_file_mmap(file, vma);
+}
+
 const struct file_operations exfat_file_operations = {
 	.llseek		= generic_file_llseek,
 	.read_iter	= generic_file_read_iter,
-	.write_iter	= generic_file_write_iter,
+	.write_iter	= exfat_file_write_iter,
 	.unlocked_ioctl = exfat_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl = exfat_compat_ioctl,
 #endif
-	.mmap		= generic_file_mmap,
+	.mmap		= exfat_file_mmap,
 	.fsync		= exfat_file_fsync,
 	.splice_read	= filemap_splice_read,
 	.splice_write	= iter_file_splice_write,
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index e7ff58b8e68c..a7bb234c8bb1 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -75,8 +75,8 @@ int __exfat_write_inode(struct inode *inode, int sync)
 	if (ei->start_clu == EXFAT_EOF_CLUSTER)
 		on_disk_size = 0;
 
-	ep2->dentry.stream.valid_size = cpu_to_le64(on_disk_size);
-	ep2->dentry.stream.size = ep2->dentry.stream.valid_size;
+	ep2->dentry.stream.valid_size = cpu_to_le64(ei->valid_size);
+	ep2->dentry.stream.size = cpu_to_le64(on_disk_size);
 	if (on_disk_size) {
 		ep2->dentry.stream.flags = ei->flags;
 		ep2->dentry.stream.start_clu = cpu_to_le32(ei->start_clu);
@@ -278,6 +278,7 @@ static int exfat_get_block(struct inode *inode, sector_t iblock,
 	unsigned int cluster, sec_offset;
 	sector_t last_block;
 	sector_t phys = 0;
+	sector_t valid_blks;
 	loff_t pos;
 
 	mutex_lock(&sbi->s_lock);
@@ -306,17 +307,32 @@ static int exfat_get_block(struct inode *inode, sector_t iblock,
 	mapped_blocks = sbi->sect_per_clus - sec_offset;
 	max_blocks = min(mapped_blocks, max_blocks);
 
-	/* Treat newly added block / cluster */
-	if (iblock < last_block)
-		create = 0;
-
-	if (create || buffer_delay(bh_result)) {
-		pos = EXFAT_BLK_TO_B((iblock + 1), sb);
+	pos = EXFAT_BLK_TO_B((iblock + 1), sb);
+	if ((create && iblock >= last_block) || buffer_delay(bh_result)) {
 		if (ei->i_size_ondisk < pos)
 			ei->i_size_ondisk = pos;
 	}
 
+	map_bh(bh_result, sb, phys);
+	if (buffer_delay(bh_result))
+		clear_buffer_delay(bh_result);
+
 	if (create) {
+		valid_blks = EXFAT_B_TO_BLK_ROUND_UP(ei->valid_size, sb);
+
+		if (iblock + max_blocks < valid_blks) {
+			/* The range has been written, map it */
+			goto done;
+		} else if (iblock < valid_blks) {
+			/*
+			 * The range has been partially written,
+			 * map the written part.
+			 */
+			max_blocks = valid_blks - iblock;
+			goto done;
+		}
+
+		/* The area has not been written, map and mark as new. */
 		err = exfat_map_new_buffer(ei, bh_result, pos);
 		if (err) {
 			exfat_fs_error(sb,
@@ -324,11 +340,55 @@ static int exfat_get_block(struct inode *inode, sector_t iblock,
 					pos, ei->i_size_aligned);
 			goto unlock_ret;
 		}
+	} else {
+		valid_blks = EXFAT_B_TO_BLK(ei->valid_size, sb);
+
+		if (iblock + max_blocks < valid_blks) {
+			/* The range has been written, map it */
+			goto done;
+		} else if (iblock < valid_blks) {
+			/*
+			 * The area has been partially written,
+			 * map the written part.
+			 */
+			max_blocks = valid_blks - iblock;
+			goto done;
+		} else if (iblock == valid_blks &&
+			   (ei->valid_size & (sb->s_blocksize - 1))) {
+			/*
+			 * The block has been partially written,
+			 * zero the unwritten part and map the block.
+			 */
+			loff_t size, off;
+
+			max_blocks = 1;
+
+			/*
+			 * For direct read, the unwritten part will be zeroed in
+			 * exfat_direct_IO()
+			 */
+			if (!bh_result->b_folio)
+				goto done;
+
+			pos -= sb->s_blocksize;
+			size = ei->valid_size - pos;
+			off = pos & (PAGE_SIZE - 1);
+
+			folio_set_bh(bh_result, bh_result->b_folio, off);
+			err = bh_read(bh_result, 0);
+			if (err < 0)
+				goto unlock_ret;
+
+			folio_zero_segment(bh_result->b_folio, off + size,
+					off + sb->s_blocksize);
+		} else {
+			/*
+			 * The range has not been written, clear the mapped flag
+			 * to only zero the cache and do not read from disk.
+			 */
+			clear_buffer_mapped(bh_result);
+		}
 	}
-
-	if (buffer_delay(bh_result))
-		clear_buffer_delay(bh_result);
-	map_bh(bh_result, sb, phys);
 done:
 	bh_result->b_size = EXFAT_BLK_TO_B(max_blocks, sb);
 unlock_ret:
@@ -343,6 +403,17 @@ static int exfat_read_folio(struct file *file, struct folio *folio)
 
 static void exfat_readahead(struct readahead_control *rac)
 {
+	struct address_space *mapping = rac->mapping;
+	struct inode *inode = mapping->host;
+	struct exfat_inode_info *ei = EXFAT_I(inode);
+	loff_t pos = readahead_pos(rac);
+
+	/* Range cross valid_size, read it page by page. */
+	if (ei->valid_size < i_size_read(inode) &&
+	    pos <= ei->valid_size &&
+	    ei->valid_size < pos + readahead_length(rac))
+		return;
+
 	mpage_readahead(rac, exfat_get_block);
 }
 
@@ -370,9 +441,7 @@ static int exfat_write_begin(struct file *file, struct address_space *mapping,
 	int ret;
 
 	*pagep = NULL;
-	ret = cont_write_begin(file, mapping, pos, len, pagep, fsdata,
-			       exfat_get_block,
-			       &EXFAT_I(mapping->host)->i_size_ondisk);
+	ret = block_write_begin(mapping, pos, len, pagep, exfat_get_block);
 
 	if (ret < 0)
 		exfat_write_failed(mapping, pos+len);
@@ -400,6 +469,11 @@ static int exfat_write_end(struct file *file, struct address_space *mapping,
 	if (err < len)
 		exfat_write_failed(mapping, pos+len);
 
+	if (!(err < 0) && pos + err > ei->valid_size) {
+		ei->valid_size = pos + err;
+		mark_inode_dirty(inode);
+	}
+
 	if (!(err < 0) && !(ei->attr & EXFAT_ATTR_ARCHIVE)) {
 		inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
 		ei->attr |= EXFAT_ATTR_ARCHIVE;
@@ -413,6 +487,8 @@ static ssize_t exfat_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 {
 	struct address_space *mapping = iocb->ki_filp->f_mapping;
 	struct inode *inode = mapping->host;
+	struct exfat_inode_info *ei = EXFAT_I(inode);
+	loff_t pos = iocb->ki_pos;
 	loff_t size = iocb->ki_pos + iov_iter_count(iter);
 	int rw = iov_iter_rw(iter);
 	ssize_t ret;
@@ -436,8 +512,21 @@ static ssize_t exfat_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 	 * condition of exfat_get_block() and ->truncate().
 	 */
 	ret = blockdev_direct_IO(iocb, inode, iter, exfat_get_block);
-	if (ret < 0 && (rw & WRITE))
-		exfat_write_failed(mapping, size);
+	if (ret < 0) {
+		if (rw & WRITE)
+			exfat_write_failed(mapping, size);
+
+		if (ret != -EIOCBQUEUED)
+			return ret;
+	} else
+		size = pos + ret;
+
+	/* zero the unwritten part in the partially written block */
+	if ((rw & READ) && pos < ei->valid_size && ei->valid_size < size) {
+		iov_iter_revert(iter, size - ei->valid_size);
+		iov_iter_zero(size - ei->valid_size, iter);
+	}
+
 	return ret;
 }
 
@@ -537,6 +626,7 @@ static int exfat_fill_inode(struct inode *inode, struct exfat_dir_entry *info)
 	ei->start_clu = info->start_clu;
 	ei->flags = info->flags;
 	ei->type = info->type;
+	ei->valid_size = info->valid_size;
 
 	ei->version = 0;
 	ei->hint_stat.eidx = 0;
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 5d737e0b639a..9c549fd11fc8 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -406,6 +406,7 @@ static int exfat_find_empty_entry(struct inode *inode,
 		i_size_write(inode, size);
 		ei->i_size_ondisk += sbi->cluster_size;
 		ei->i_size_aligned += sbi->cluster_size;
+		ei->valid_size += sbi->cluster_size;
 		ei->flags = p_dir->flags;
 		inode->i_blocks += sbi->cluster_size >> 9;
 	}
@@ -558,6 +559,8 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 		info->size = clu_size;
 		info->num_subdirs = EXFAT_MIN_SUBDIR;
 	}
+	info->valid_size = info->size;
+
 	memset(&info->crtime, 0, sizeof(info->crtime));
 	memset(&info->mtime, 0, sizeof(info->mtime));
 	memset(&info->atime, 0, sizeof(info->atime));
@@ -660,6 +663,8 @@ static int exfat_find(struct inode *dir, struct qstr *qname,
 	info->type = exfat_get_entry_type(ep);
 	info->attr = le16_to_cpu(ep->dentry.file.attr);
 	info->size = le64_to_cpu(ep2->dentry.stream.valid_size);
+	info->valid_size = le64_to_cpu(ep2->dentry.stream.valid_size);
+	info->size = le64_to_cpu(ep2->dentry.stream.size);
 	if (info->size == 0) {
 		info->flags = ALLOC_NO_FAT_CHAIN;
 		info->start_clu = EXFAT_EOF_CLUSTER;
@@ -1288,6 +1293,7 @@ static int __exfat_rename(struct inode *old_parent_inode,
 			}
 
 			i_size_write(new_inode, 0);
+			new_ei->valid_size = 0;
 			new_ei->start_clu = EXFAT_EOF_CLUSTER;
 			new_ei->flags = ALLOC_NO_FAT_CHAIN;
 		}
-- 
2.25.1

Re: [PATCH v5 1/2] exfat: change to get file size from DataLength

[kernel test robot]

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.7-rc3 next-20231130]
[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/Yuezhang-Mo-sony-com/exfat-do-not-zero-the-extended-part/20231130-164222
base:   linus/master
patch link:    https://lore.kernel.org/r/PUZPR04MB6316F0640983B00CC55D903F8182A%40PUZPR04MB6316.apcprd04.prod.outlook.com
patch subject: [PATCH v5 1/2] exfat: change to get file size from DataLength
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20231201/202312010044.6CIJOsWq-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231201/202312010044.6CIJOsWq-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/202312010044.6CIJOsWq-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/exfat/file.c:543:22: warning: format specifies type 'long' but the argument has type 'ssize_t' (aka 'int') [-Wformat]
                                   valid_size, pos, ret);
                                                    ^~~
   fs/exfat/exfat_fs.h:545:51: note: expanded from macro 'exfat_err'
           pr_err("exFAT-fs (%s): " fmt "\n", (sb)->s_id, ##__VA_ARGS__)
                                    ~~~                     ^~~~~~~~~~~
   include/linux/printk.h:498:33: note: expanded from macro 'pr_err'
           printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
                                  ~~~     ^~~~~~~~~~~
   include/linux/printk.h:455:60: note: expanded from macro 'printk'
   #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
                                                       ~~~    ^~~~~~~~~~~
   include/linux/printk.h:427:19: note: expanded from macro 'printk_index_wrap'
                   _p_func(_fmt, ##__VA_ARGS__);                           \
                           ~~~~    ^~~~~~~~~~~
   1 warning generated.


vim +543 fs/exfat/file.c

   520	
   521	static ssize_t exfat_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
   522	{
   523		ssize_t ret;
   524		struct file *file = iocb->ki_filp;
   525		struct inode *inode = file_inode(file);
   526		struct exfat_inode_info *ei = EXFAT_I(inode);
   527		loff_t pos = iocb->ki_pos;
   528		loff_t valid_size;
   529	
   530		inode_lock(inode);
   531	
   532		valid_size = ei->valid_size;
   533	
   534		ret = generic_write_checks(iocb, iter);
   535		if (ret < 0)
   536			goto unlock;
   537	
   538		if (pos > valid_size) {
   539			ret = exfat_file_zeroed_range(file, valid_size, pos);
   540			if (ret < 0 && ret != -ENOSPC) {
   541				exfat_err(inode->i_sb,
   542					"write: fail to zero from %llu to %llu(%ld)",
 > 543					valid_size, pos, ret);
   544			}
   545			if (ret < 0)
   546				goto unlock;
   547		}
   548	
   549		ret = __generic_file_write_iter(iocb, iter);
   550		if (ret < 0)
   551			goto unlock;
   552	
   553		inode_unlock(inode);
   554	
   555		if (pos > valid_size)
   556			pos = valid_size;
   557	
   558		if (iocb_is_dsync(iocb) && iocb->ki_pos > pos) {
   559			ssize_t err = vfs_fsync_range(file, pos, iocb->ki_pos - 1,
   560					iocb->ki_flags & IOCB_SYNC);
   561			if (err < 0)
   562				return err;
   563		}
   564	
   565		return ret;
   566	
   567	unlock:
   568		inode_unlock(inode);
   569	
   570		return ret;
   571	}
   572	

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

Re: [PATCH v5 1/2] exfat: change to get file size from DataLength

[kernel test robot]

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.7-rc3 next-20231130]
[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/Yuezhang-Mo-sony-com/exfat-do-not-zero-the-extended-part/20231130-164222
base:   linus/master
patch link:    https://lore.kernel.org/r/PUZPR04MB6316F0640983B00CC55D903F8182A%40PUZPR04MB6316.apcprd04.prod.outlook.com
patch subject: [PATCH v5 1/2] exfat: change to get file size from DataLength
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20231201/202312010116.gCpHjGSB-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231201/202312010116.gCpHjGSB-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/202312010116.gCpHjGSB-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:22,
                    from arch/m68k/include/asm/bug.h:32,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/m68k/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:79,
                    from include/linux/spinlock.h:56,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/slab.h:16,
                    from fs/exfat/file.c:6:
   fs/exfat/file.c: In function 'exfat_file_write_iter':
>> include/linux/kern_levels.h:5:25: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'ssize_t' {aka 'int'} [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:427:25: note: in definition of macro 'printk_index_wrap'
     427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:498:9: note: in expansion of macro 'printk'
     498 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR        KERN_SOH "3"    /* error conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:498:16: note: in expansion of macro 'KERN_ERR'
     498 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~
   fs/exfat/exfat_fs.h:545:9: note: in expansion of macro 'pr_err'
     545 |         pr_err("exFAT-fs (%s): " fmt "\n", (sb)->s_id, ##__VA_ARGS__)
         |         ^~~~~~
   fs/exfat/file.c:541:25: note: in expansion of macro 'exfat_err'
     541 |                         exfat_err(inode->i_sb,
         |                         ^~~~~~~~~


vim +5 include/linux/kern_levels.h

314ba3520e513a Joe Perches 2012-07-30  4  
04d2c8c83d0e3a Joe Perches 2012-07-30 @5  #define KERN_SOH	"\001"		/* ASCII Start Of Header */
04d2c8c83d0e3a Joe Perches 2012-07-30  6  #define KERN_SOH_ASCII	'\001'
04d2c8c83d0e3a Joe Perches 2012-07-30  7  

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

Re: [PATCH v5 1/2] exfat: change to get file size from DataLength

[Dan Carpenter]

Hi,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yuezhang-Mo-sony-com/exfat-do-not-zero-the-extended-part/20231130-164222
base:   linus/master
patch link:    https://lore.kernel.org/r/PUZPR04MB6316F0640983B00CC55D903F8182A%40PUZPR04MB6316.apcprd04.prod.outlook.com
patch subject: [PATCH v5 1/2] exfat: change to get file size from DataLength
config: x86_64-randconfig-r081-20231130 (https://download.01.org/0day-ci/archive/20231201/202312010428.73gtRyvj-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20231201/202312010428.73gtRyvj-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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202312010428.73gtRyvj-lkp@intel.com/

smatch warnings:
fs/exfat/inode.c:525 exfat_direct_IO() warn: bitwise AND condition is false here

vim +525 fs/exfat/inode.c

5f2aa075070cf5b Namjae Jeon          2020-03-02  486  static ssize_t exfat_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
5f2aa075070cf5b Namjae Jeon          2020-03-02  487  {
5f2aa075070cf5b Namjae Jeon          2020-03-02  488  	struct address_space *mapping = iocb->ki_filp->f_mapping;
5f2aa075070cf5b Namjae Jeon          2020-03-02  489  	struct inode *inode = mapping->host;
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  490  	struct exfat_inode_info *ei = EXFAT_I(inode);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  491  	loff_t pos = iocb->ki_pos;
5f2aa075070cf5b Namjae Jeon          2020-03-02  492  	loff_t size = iocb->ki_pos + iov_iter_count(iter);
5f2aa075070cf5b Namjae Jeon          2020-03-02  493  	int rw = iov_iter_rw(iter);
5f2aa075070cf5b Namjae Jeon          2020-03-02  494  	ssize_t ret;
5f2aa075070cf5b Namjae Jeon          2020-03-02  495  
5f2aa075070cf5b Namjae Jeon          2020-03-02  496  	if (rw == WRITE) {
5f2aa075070cf5b Namjae Jeon          2020-03-02  497  		/*
5f2aa075070cf5b Namjae Jeon          2020-03-02  498  		 * FIXME: blockdev_direct_IO() doesn't use ->write_begin(),
5f2aa075070cf5b Namjae Jeon          2020-03-02  499  		 * so we need to update the ->i_size_aligned to block boundary.
5f2aa075070cf5b Namjae Jeon          2020-03-02  500  		 *
5f2aa075070cf5b Namjae Jeon          2020-03-02  501  		 * But we must fill the remaining area or hole by nul for
5f2aa075070cf5b Namjae Jeon          2020-03-02  502  		 * updating ->i_size_aligned
5f2aa075070cf5b Namjae Jeon          2020-03-02  503  		 *
5f2aa075070cf5b Namjae Jeon          2020-03-02  504  		 * Return 0, and fallback to normal buffered write.
5f2aa075070cf5b Namjae Jeon          2020-03-02  505  		 */
5f2aa075070cf5b Namjae Jeon          2020-03-02  506  		if (EXFAT_I(inode)->i_size_aligned < size)
5f2aa075070cf5b Namjae Jeon          2020-03-02  507  			return 0;
5f2aa075070cf5b Namjae Jeon          2020-03-02  508  	}
5f2aa075070cf5b Namjae Jeon          2020-03-02  509  
5f2aa075070cf5b Namjae Jeon          2020-03-02  510  	/*
5f2aa075070cf5b Namjae Jeon          2020-03-02  511  	 * Need to use the DIO_LOCKING for avoiding the race
5f2aa075070cf5b Namjae Jeon          2020-03-02  512  	 * condition of exfat_get_block() and ->truncate().
5f2aa075070cf5b Namjae Jeon          2020-03-02  513  	 */
5f2aa075070cf5b Namjae Jeon          2020-03-02  514  	ret = blockdev_direct_IO(iocb, inode, iter, exfat_get_block);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  515  	if (ret < 0) {
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  516  		if (rw & WRITE)

This code works and the checker doesn't complain about it, but for
consistency I think it should be if (rw == WRITE).

5f2aa075070cf5b Namjae Jeon          2020-03-02  517  			exfat_write_failed(mapping, size);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  518  
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  519  		if (ret != -EIOCBQUEUED)
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  520  			return ret;
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  521  	} else
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  522  		size = pos + ret;
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  523  
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  524  	/* zero the unwritten part in the partially written block */
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30 @525  	if ((rw & READ) && pos < ei->valid_size && ei->valid_size < size) {

I think this should be rw == READ.

6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  526  		iov_iter_revert(iter, size - ei->valid_size);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  527  		iov_iter_zero(size - ei->valid_size, iter);
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  528  	}
6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  529  
5f2aa075070cf5b Namjae Jeon          2020-03-02  530  	return ret;
5f2aa075070cf5b Namjae Jeon          2020-03-02  531  }

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

RE: [PATCH v5 1/2] exfat: change to get file size from DataLength

[Sungjong Seo]

[snip]
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  515  	if (ret < 0)
{
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  516  		if
(rw
> & WRITE)
> 
> This code works and the checker doesn't complain about it, but for
> consistency I think it should be if (rw == WRITE).
> 
> 5f2aa075070cf5b Namjae Jeon          2020-03-02  517
> 	exfat_write_failed(mapping, size);
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  518
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  519  		if
> (ret != -EIOCBQUEUED)
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  520
> 	return ret;
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  521  	} else
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  522  		size
=
> pos + ret;
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  523
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30  524  	/* zero the
> unwritten part in the partially written block */
> 6642222a5afe775 Yuezhang.Mo@sony.com 2023-11-30 @525  	if ((rw &
READ)
> && pos < ei->valid_size && ei->valid_size < size) {
> 
> I think this should be rw == READ.
You're definitely right.
READ is 0, so it always be false.

Dear Yuezhang,

Can you please send v6 again for this?
It would be nice to include fixes for a minor issue reported
by Kernel test robot.

Thanks

Re: [PATCH v5 1/2] exfat: change to get file size from DataLength

[Namjae Jeon]

> +static int exfat_file_zeroed_range(struct file *file, loff_t start, loff_t
> end)
> +{
> +	int err;
> +	struct inode *inode = file_inode(file);
> +	struct exfat_inode_info *ei = EXFAT_I(inode);
> +	struct address_space *mapping = inode->i_mapping;
> +	const struct address_space_operations *ops = mapping->a_ops;
> +
> +	while (start < end) {
> +		u32 zerofrom, len;
> +		struct page *page = NULL;
> +
> +		zerofrom = start & (PAGE_SIZE - 1);
> +		len = PAGE_SIZE - zerofrom;
> +		if (start + len > end)
> +			len = end - start;
> +
> +		err = ops->write_begin(file, mapping, start, len, &page, NULL);
Is there any reason why you don't use block_write_begin and
generic_write_end() ?

Thanks.
> +		if (err)
> +			goto out;
> +
> +		zero_user_segment(page, zerofrom, zerofrom + len);
> +
> +		err = ops->write_end(file, mapping, start, len, len, page, NULL);
> +		if (err < 0)
> +			goto out;
> +		start += len;
> +
> +		balance_dirty_pages_ratelimited(mapping);
> +		cond_resched();
> +	}
> +
> +	ei->valid_size = end;
> +	mark_inode_dirty(inode);
> +
> +out:
> +	return err;
> +}

RE: [PATCH v5 1/2] exfat: change to get file size from DataLength

[Yuezhang.Mo]

> -----Original Message-----
> From: Namjae Jeon <linkinjeon@kernel.org>
> Sent: Tuesday, December 5, 2023 11:30 AM
> To: Mo, Yuezhang <Yuezhang.Mo@sony.com>
> Cc: sj1557.seo@samsung.com; linux-fsdevel@vger.kernel.org; Wu, Andy
> <Andy.Wu@sony.com>; Aoyama, Wataru (SGC) <Wataru.Aoyama@sony.com>;
> cpgs@samsung.com
> Subject: Re: [PATCH v5 1/2] exfat: change to get file size from DataLength
> 
> > +static int exfat_file_zeroed_range(struct file *file, loff_t start, loff_t
> > end)
> > +{
> > +	int err;
> > +	struct inode *inode = file_inode(file);
> > +	struct exfat_inode_info *ei = EXFAT_I(inode);
> > +	struct address_space *mapping = inode->i_mapping;
> > +	const struct address_space_operations *ops = mapping->a_ops;
> > +
> > +	while (start < end) {
> > +		u32 zerofrom, len;
> > +		struct page *page = NULL;
> > +
> > +		zerofrom = start & (PAGE_SIZE - 1);
> > +		len = PAGE_SIZE - zerofrom;
> > +		if (start + len > end)
> > +			len = end - start;
> > +
> > +		err = ops->write_begin(file, mapping, start, len, &page, NULL);
> Is there any reason why you don't use block_write_begin and
> generic_write_end() ?
> 
> Thanks.

If use block_write_begin(), we need to remove 'static' from exfat_get_block(),
use ops->write_begin() and ops->write_end() makes the function more generic,
maybe we can rename this function to generic_write_zero() and move it to
fs/buffer.c.

And ei->valid_size had updated in ops->write_end(), it is unneeded to update in
this function.

> > +		if (err)
> > +			goto out;
> > +
> > +		zero_user_segment(page, zerofrom, zerofrom + len);
> > +
> > +		err = ops->write_end(file, mapping, start, len, len, page, NULL);
> > +		if (err < 0)
> > +			goto out;
> > +		start += len;
> > +
> > +		balance_dirty_pages_ratelimited(mapping);
> > +		cond_resched();
> > +	}
> > +
> > +	ei->valid_size = end;
> > +	mark_inode_dirty(inode);
> > +
> > +out:
> > +	return err;
> > +}