Linux-Fsdevel Archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] exfat: get file size from DataLength
@ 2023-11-02  9:58 Yuezhang.Mo
  2023-11-30  3:09 ` [PATCH v5 " Yuezhang.Mo
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Yuezhang.Mo @ 2023-11-02  9:58 UTC (permalink / raw
  To: linkinjeon@kernel.org, sj1557.seo@samsung.com
  Cc: linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.com

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

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

* [PATCH v5 0/2] exfat: get file size from DataLength
  2023-11-02  9:58 [PATCH v4 0/2] exfat: get file size from DataLength Yuezhang.Mo
@ 2023-11-30  3:09 ` Yuezhang.Mo
  2023-12-05 10:16   ` [PATCH v6 " Yuezhang.Mo
                     ` (2 more replies)
  2023-11-30  3:09 ` [PATCH v5 1/2] exfat: change to get file size from DataLength Yuezhang.Mo
  2023-11-30  3:09 ` [PATCH v5 2/2] exfat: do not zero the extended part Yuezhang.Mo
  2 siblings, 3 replies; 16+ messages in thread
From: Yuezhang.Mo @ 2023-11-30  3:09 UTC (permalink / raw
  To: linkinjeon@kernel.org, sj1557.seo@samsung.com
  Cc: linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.com, cpgs@samsung.com

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 v5:
  - do not call exfat_map_new_buffer() if iblock + max_blocks < valid_blks.
  - Reorganized the logic of exfat_get_block(), both writing and reading use
    block index judgment.
  - Remove unnecessary code moves.
  - Reduce sync in exfat_file_write_iter()

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 zero the extended part

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

-- 
2.25.1

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

* [PATCH v5 1/2] exfat: change to get file size from DataLength
  2023-11-02  9:58 [PATCH v4 0/2] exfat: get file size from DataLength Yuezhang.Mo
  2023-11-30  3:09 ` [PATCH v5 " Yuezhang.Mo
@ 2023-11-30  3:09 ` Yuezhang.Mo
  2023-11-30 17:10   ` kernel test robot
                     ` (3 more replies)
  2023-11-30  3:09 ` [PATCH v5 2/2] exfat: do not zero the extended part Yuezhang.Mo
  2 siblings, 4 replies; 16+ messages in thread
From: Yuezhang.Mo @ 2023-11-30  3:09 UTC (permalink / raw
  To: linkinjeon@kernel.org, sj1557.seo@samsung.com
  Cc: linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.com, cpgs@samsung.com

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

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

* [PATCH v5 2/2] exfat: do not zero the extended part
  2023-11-02  9:58 [PATCH v4 0/2] exfat: get file size from DataLength Yuezhang.Mo
  2023-11-30  3:09 ` [PATCH v5 " Yuezhang.Mo
  2023-11-30  3:09 ` [PATCH v5 1/2] exfat: change to get file size from DataLength Yuezhang.Mo
@ 2023-11-30  3:09 ` Yuezhang.Mo
  2 siblings, 0 replies; 16+ messages in thread
From: Yuezhang.Mo @ 2023-11-30  3:09 UTC (permalink / raw
  To: linkinjeon@kernel.org, sj1557.seo@samsung.com
  Cc: linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.com, cpgs@samsung.com

Since the read operation beyond the ValidDataLength returns zero,
if we just extend the size of the file, we don't need to zero the
extended part, but only change the DataLength without changing
the ValidDataLength.

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/file.c  | 77 +++++++++++++++++++++++++++++++++++-------------
 fs/exfat/inode.c | 14 ++++++++-
 2 files changed, 70 insertions(+), 21 deletions(-)

diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 154f39a03a69..3d01a4eefc11 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -18,32 +18,69 @@
 
 static int exfat_cont_expand(struct inode *inode, loff_t size)
 {
-	struct address_space *mapping = inode->i_mapping;
-	loff_t start = i_size_read(inode), count = size - i_size_read(inode);
-	int err, err2;
+	int ret;
+	unsigned int num_clusters, new_num_clusters, last_clu;
+	struct exfat_inode_info *ei = EXFAT_I(inode);
+	struct super_block *sb = inode->i_sb;
+	struct exfat_sb_info *sbi = EXFAT_SB(sb);
+	struct exfat_chain clu;
 
-	err = generic_cont_expand_simple(inode, size);
-	if (err)
-		return err;
+	ret = inode_newsize_ok(inode, size);
+	if (ret)
+		return ret;
+
+	num_clusters = EXFAT_B_TO_CLU_ROUND_UP(ei->i_size_ondisk, sbi);
+	new_num_clusters = EXFAT_B_TO_CLU_ROUND_UP(size, sbi);
+
+	if (new_num_clusters == num_clusters)
+		goto out;
+
+	exfat_chain_set(&clu, ei->start_clu, num_clusters, ei->flags);
+	ret = exfat_find_last_cluster(sb, &clu, &last_clu);
+	if (ret)
+		return ret;
 
+	clu.dir = (last_clu == EXFAT_EOF_CLUSTER) ?
+			EXFAT_EOF_CLUSTER : last_clu + 1;
+	clu.size = 0;
+	clu.flags = ei->flags;
+
+	ret = exfat_alloc_cluster(inode, new_num_clusters - num_clusters,
+			&clu, IS_DIRSYNC(inode));
+	if (ret)
+		return ret;
+
+	/* Append new clusters to chain */
+	if (clu.flags != ei->flags) {
+		exfat_chain_cont_cluster(sb, ei->start_clu, num_clusters);
+		ei->flags = ALLOC_FAT_CHAIN;
+	}
+	if (clu.flags == ALLOC_FAT_CHAIN)
+		if (exfat_ent_set(sb, last_clu, clu.dir))
+			goto free_clu;
+
+	if (num_clusters == 0)
+		ei->start_clu = clu.dir;
+
+out:
 	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
-	EXFAT_I(inode)->valid_size = size;
-	mark_inode_dirty(inode);
+	/* Expanded range not zeroed, do not update valid_size */
+	i_size_write(inode, size);
 
-	if (!IS_SYNC(inode))
-		return 0;
+	ei->i_size_aligned = round_up(size, sb->s_blocksize);
+	ei->i_size_ondisk = ei->i_size_aligned;
+	inode->i_blocks = round_up(size, sbi->cluster_size) >> 9;
 
-	err = filemap_fdatawrite_range(mapping, start, start + count - 1);
-	err2 = sync_mapping_buffers(mapping);
-	if (!err)
-		err = err2;
-	err2 = write_inode_now(inode, 1);
-	if (!err)
-		err = err2;
-	if (err)
-		return err;
+	if (IS_DIRSYNC(inode))
+		return write_inode_now(inode, 1);
+
+	mark_inode_dirty(inode);
+
+	return 0;
 
-	return filemap_fdatawait_range(mapping, start, start + count - 1);
+free_clu:
+	exfat_free_cluster(inode, &clu);
+	return -EIO;
 }
 
 static bool exfat_allow_set_time(struct exfat_sb_info *sbi, struct inode *inode)
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index a7bb234c8bb1..9793c64f4ad6 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -75,8 +75,17 @@ 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(ei->valid_size);
 	ep2->dentry.stream.size = cpu_to_le64(on_disk_size);
+	/*
+	 * mmap write does not use exfat_write_end(), valid_size may be
+	 * extended to the sector-aligned length in exfat_get_block().
+	 * So we need to fixup valid_size to the writren length.
+	 */
+	if (on_disk_size < ei->valid_size)
+		ep2->dentry.stream.valid_size = ep2->dentry.stream.size;
+	else
+		ep2->dentry.stream.valid_size = cpu_to_le64(ei->valid_size);
+
 	if (on_disk_size) {
 		ep2->dentry.stream.flags = ei->flags;
 		ep2->dentry.stream.start_clu = cpu_to_le32(ei->start_clu);
@@ -340,6 +349,9 @@ static int exfat_get_block(struct inode *inode, sector_t iblock,
 					pos, ei->i_size_aligned);
 			goto unlock_ret;
 		}
+
+		ei->valid_size = EXFAT_BLK_TO_B(iblock + max_blocks, sb);
+		mark_inode_dirty(inode);
 	} else {
 		valid_blks = EXFAT_B_TO_BLK(ei->valid_size, sb);
 
-- 
2.25.1


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

* Re: [PATCH v5 1/2] exfat: change to get file size from DataLength
  2023-11-30  3:09 ` [PATCH v5 1/2] exfat: change to get file size from DataLength Yuezhang.Mo
@ 2023-11-30 17:10   ` kernel test robot
  2023-11-30 18:04   ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-11-30 17:10 UTC (permalink / raw
  To: Yuezhang.Mo@sony.com, linkinjeon@kernel.org,
	sj1557.seo@samsung.com
  Cc: llvm, oe-kbuild-all, linux-fsdevel@vger.kernel.org,
	Andy.Wu@sony.com, Wataru.Aoyama@sony.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
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

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

* Re: [PATCH v5 1/2] exfat: change to get file size from DataLength
  2023-11-30  3:09 ` [PATCH v5 1/2] exfat: change to get file size from DataLength Yuezhang.Mo
  2023-11-30 17:10   ` kernel test robot
@ 2023-11-30 18:04   ` kernel test robot
  2023-12-01  8:29   ` Dan Carpenter
  2023-12-05  3:30   ` Namjae Jeon
  3 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-11-30 18:04 UTC (permalink / raw
  To: Yuezhang.Mo@sony.com, linkinjeon@kernel.org,
	sj1557.seo@samsung.com
  Cc: oe-kbuild-all, linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.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
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

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

* Re: [PATCH v5 1/2] exfat: change to get file size from DataLength
  2023-11-30  3:09 ` [PATCH v5 1/2] exfat: change to get file size from DataLength Yuezhang.Mo
  2023-11-30 17:10   ` kernel test robot
  2023-11-30 18:04   ` kernel test robot
@ 2023-12-01  8:29   ` Dan Carpenter
  2023-12-05  2:11     ` Sungjong Seo
  2023-12-05  3:30   ` Namjae Jeon
  3 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2023-12-01  8:29 UTC (permalink / raw
  To: oe-kbuild, Yuezhang.Mo@sony.com, linkinjeon@kernel.org,
	sj1557.seo@samsung.com
  Cc: lkp, oe-kbuild-all, linux-fsdevel@vger.kernel.org,
	Andy.Wu@sony.com, Wataru.Aoyama@sony.com, cpgs@samsung.com

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


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

* RE: [PATCH v5 1/2] exfat: change to get file size from DataLength
  2023-12-01  8:29   ` Dan Carpenter
@ 2023-12-05  2:11     ` Sungjong Seo
  0 siblings, 0 replies; 16+ messages in thread
From: Sungjong Seo @ 2023-12-05  2:11 UTC (permalink / raw
  To: 'Dan Carpenter', oe-kbuild, Yuezhang.Mo, linkinjeon
  Cc: lkp, oe-kbuild-all, linux-fsdevel, Andy.Wu, Wataru.Aoyama, cpgs,
	sj1557.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



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

* Re: [PATCH v5 1/2] exfat: change to get file size from DataLength
  2023-11-30  3:09 ` [PATCH v5 1/2] exfat: change to get file size from DataLength Yuezhang.Mo
                     ` (2 preceding siblings ...)
  2023-12-01  8:29   ` Dan Carpenter
@ 2023-12-05  3:30   ` Namjae Jeon
       [not found]     ` <PUZPR04MB6316B8BAC361A5B2A70FD5098185A@PUZPR04MB6316.apcprd04.prod.outlook.com>
  3 siblings, 1 reply; 16+ messages in thread
From: Namjae Jeon @ 2023-12-05  3:30 UTC (permalink / raw
  To: Yuezhang.Mo@sony.com
  Cc: sj1557.seo@samsung.com, linux-fsdevel@vger.kernel.org,
	Andy.Wu@sony.com, Wataru.Aoyama@sony.com, cpgs@samsung.com

> +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;
> +}

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

* RE: [PATCH v5 1/2] exfat: change to get file size from DataLength
       [not found]     ` <PUZPR04MB6316B8BAC361A5B2A70FD5098185A@PUZPR04MB6316.apcprd04.prod.outlook.com>
@ 2023-12-05  5:29       ` Yuezhang.Mo
  0 siblings, 0 replies; 16+ messages in thread
From: Yuezhang.Mo @ 2023-12-05  5:29 UTC (permalink / raw
  To: linkinjeon@kernel.org, sj1557.seo@samsung.com
  Cc: linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.com, cpgs@samsung.com

> -----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;
> > +}

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

* [PATCH v6 0/2] exfat: get file size from DataLength
  2023-11-30  3:09 ` [PATCH v5 " Yuezhang.Mo
@ 2023-12-05 10:16   ` Yuezhang.Mo
  2023-12-12  4:12     ` Namjae Jeon
  2023-12-05 10:16   ` [PATCH v6 1/2] exfat: change to " Yuezhang.Mo
  2023-12-05 10:16   ` [PATCH v6 2/2] exfat: do not zero the extended part Yuezhang.Mo
  2 siblings, 1 reply; 16+ messages in thread
From: Yuezhang.Mo @ 2023-12-05 10:16 UTC (permalink / raw
  To: linkinjeon@kernel.org, sj1557.seo@samsung.com
  Cc: linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.com, cpgs@samsung.com, Dan Carpenter

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 v6:
  - Fix build warning of printk() on 32-bit system.
  - Fix read/write judgment in exfat_direct_IO().
  - Remove update ei->valid_size in exfat_file_zeroed_range().

Changes for v5:
  - do not call exfat_map_new_buffer() if iblock + max_blocks < valid_blks.
  - Reorganized the logic of exfat_get_block(), both writing and reading use
    block index judgment.
  - Remove unnecessary code moves.
  - Reduce sync in exfat_file_write_iter()

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 zero the extended part

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

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

-- 
2.25.1

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

* [PATCH v6 1/2] exfat: change to get file size from DataLength
  2023-11-30  3:09 ` [PATCH v5 " Yuezhang.Mo
  2023-12-05 10:16   ` [PATCH v6 " Yuezhang.Mo
@ 2023-12-05 10:16   ` Yuezhang.Mo
  2023-12-05 10:16   ` [PATCH v6 2/2] exfat: do not zero the extended part Yuezhang.Mo
  2 siblings, 0 replies; 16+ messages in thread
From: Yuezhang.Mo @ 2023-12-05 10:16 UTC (permalink / raw
  To: linkinjeon@kernel.org, sj1557.seo@samsung.com
  Cc: linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.com, cpgs@samsung.com, Dan Carpenter

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     | 118 ++++++++++++++++++++++++++++++++++++++++-
 fs/exfat/inode.c    | 124 ++++++++++++++++++++++++++++++++++++++------
 fs/exfat/namei.c    |   6 +++
 4 files changed, 231 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..270e2f934124 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,124 @@ 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 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();
+	}
+
+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(%zd)",
+				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..b02677c9fd45 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

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

* [PATCH v6 2/2] exfat: do not zero the extended part
  2023-11-30  3:09 ` [PATCH v5 " Yuezhang.Mo
  2023-12-05 10:16   ` [PATCH v6 " Yuezhang.Mo
  2023-12-05 10:16   ` [PATCH v6 1/2] exfat: change to " Yuezhang.Mo
@ 2023-12-05 10:16   ` Yuezhang.Mo
  2 siblings, 0 replies; 16+ messages in thread
From: Yuezhang.Mo @ 2023-12-05 10:16 UTC (permalink / raw
  To: linkinjeon@kernel.org, sj1557.seo@samsung.com
  Cc: linux-fsdevel@vger.kernel.org, Andy.Wu@sony.com,
	Wataru.Aoyama@sony.com, cpgs@samsung.com, Dan Carpenter

Since the read operation beyond the ValidDataLength returns zero,
if we just extend the size of the file, we don't need to zero the
extended part, but only change the DataLength without changing
the ValidDataLength.

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/file.c  | 77 +++++++++++++++++++++++++++++++++++-------------
 fs/exfat/inode.c | 14 ++++++++-
 2 files changed, 70 insertions(+), 21 deletions(-)

diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 270e2f934124..d25a96a148af 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -18,32 +18,69 @@
 
 static int exfat_cont_expand(struct inode *inode, loff_t size)
 {
-	struct address_space *mapping = inode->i_mapping;
-	loff_t start = i_size_read(inode), count = size - i_size_read(inode);
-	int err, err2;
+	int ret;
+	unsigned int num_clusters, new_num_clusters, last_clu;
+	struct exfat_inode_info *ei = EXFAT_I(inode);
+	struct super_block *sb = inode->i_sb;
+	struct exfat_sb_info *sbi = EXFAT_SB(sb);
+	struct exfat_chain clu;
 
-	err = generic_cont_expand_simple(inode, size);
-	if (err)
-		return err;
+	ret = inode_newsize_ok(inode, size);
+	if (ret)
+		return ret;
+
+	num_clusters = EXFAT_B_TO_CLU_ROUND_UP(ei->i_size_ondisk, sbi);
+	new_num_clusters = EXFAT_B_TO_CLU_ROUND_UP(size, sbi);
+
+	if (new_num_clusters == num_clusters)
+		goto out;
+
+	exfat_chain_set(&clu, ei->start_clu, num_clusters, ei->flags);
+	ret = exfat_find_last_cluster(sb, &clu, &last_clu);
+	if (ret)
+		return ret;
 
+	clu.dir = (last_clu == EXFAT_EOF_CLUSTER) ?
+			EXFAT_EOF_CLUSTER : last_clu + 1;
+	clu.size = 0;
+	clu.flags = ei->flags;
+
+	ret = exfat_alloc_cluster(inode, new_num_clusters - num_clusters,
+			&clu, IS_DIRSYNC(inode));
+	if (ret)
+		return ret;
+
+	/* Append new clusters to chain */
+	if (clu.flags != ei->flags) {
+		exfat_chain_cont_cluster(sb, ei->start_clu, num_clusters);
+		ei->flags = ALLOC_FAT_CHAIN;
+	}
+	if (clu.flags == ALLOC_FAT_CHAIN)
+		if (exfat_ent_set(sb, last_clu, clu.dir))
+			goto free_clu;
+
+	if (num_clusters == 0)
+		ei->start_clu = clu.dir;
+
+out:
 	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
-	EXFAT_I(inode)->valid_size = size;
-	mark_inode_dirty(inode);
+	/* Expanded range not zeroed, do not update valid_size */
+	i_size_write(inode, size);
 
-	if (!IS_SYNC(inode))
-		return 0;
+	ei->i_size_aligned = round_up(size, sb->s_blocksize);
+	ei->i_size_ondisk = ei->i_size_aligned;
+	inode->i_blocks = round_up(size, sbi->cluster_size) >> 9;
 
-	err = filemap_fdatawrite_range(mapping, start, start + count - 1);
-	err2 = sync_mapping_buffers(mapping);
-	if (!err)
-		err = err2;
-	err2 = write_inode_now(inode, 1);
-	if (!err)
-		err = err2;
-	if (err)
-		return err;
+	if (IS_DIRSYNC(inode))
+		return write_inode_now(inode, 1);
+
+	mark_inode_dirty(inode);
+
+	return 0;
 
-	return filemap_fdatawait_range(mapping, start, start + count - 1);
+free_clu:
+	exfat_free_cluster(inode, &clu);
+	return -EIO;
 }
 
 static bool exfat_allow_set_time(struct exfat_sb_info *sbi, struct inode *inode)
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index b02677c9fd45..522edcbb2ce4 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -75,8 +75,17 @@ 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(ei->valid_size);
 	ep2->dentry.stream.size = cpu_to_le64(on_disk_size);
+	/*
+	 * mmap write does not use exfat_write_end(), valid_size may be
+	 * extended to the sector-aligned length in exfat_get_block().
+	 * So we need to fixup valid_size to the writren length.
+	 */
+	if (on_disk_size < ei->valid_size)
+		ep2->dentry.stream.valid_size = ep2->dentry.stream.size;
+	else
+		ep2->dentry.stream.valid_size = cpu_to_le64(ei->valid_size);
+
 	if (on_disk_size) {
 		ep2->dentry.stream.flags = ei->flags;
 		ep2->dentry.stream.start_clu = cpu_to_le32(ei->start_clu);
@@ -340,6 +349,9 @@ static int exfat_get_block(struct inode *inode, sector_t iblock,
 					pos, ei->i_size_aligned);
 			goto unlock_ret;
 		}
+
+		ei->valid_size = EXFAT_BLK_TO_B(iblock + max_blocks, sb);
+		mark_inode_dirty(inode);
 	} else {
 		valid_blks = EXFAT_B_TO_BLK(ei->valid_size, sb);
 
-- 
2.25.1


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

* Re: [PATCH v6 0/2] exfat: get file size from DataLength
  2023-12-05 10:16   ` [PATCH v6 " Yuezhang.Mo
@ 2023-12-12  4:12     ` Namjae Jeon
       [not found]       ` <PUZPR04MB63160A6FD8E7EF04E4342B1B818EA@PUZPR04MB6316.apcprd04.prod.outlook.com>
  0 siblings, 1 reply; 16+ messages in thread
From: Namjae Jeon @ 2023-12-12  4:12 UTC (permalink / raw
  To: Yuezhang.Mo@sony.com
  Cc: sj1557.seo@samsung.com, linux-fsdevel@vger.kernel.org,
	Andy.Wu@sony.com, Wataru.Aoyama@sony.com, cpgs@samsung.com,
	Dan Carpenter

2023-12-05 19:16 GMT+09:00, Yuezhang.Mo@sony.com <Yuezhang.Mo@sony.com>:
> 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.
Have you ever run xfstests against v6 version ?
The page allocation failure problem in my test happen below.
What is your test result?

[ 3029.994963] run fstests generic/476 at 2023-12-12 09:39:35
[12514.707396] perf: interrupt took too long (2501 > 2500), lowering
kernel.perf_event_max_sample_rate to 79750
[13670.913452] fsstress: page allocation failure: order:0,
mode:0x40808(GFP_NOWAIT|__GFP_COMP|__GFP_MOVABLE),
nodemask=(null),cpuset=/,mems_allowed=0
[13670.913470] CPU: 0 PID: 297913 Comm: fsstress Tainted: G
OE      6.7.0-rc4 #1
[13670.913474] Hardware name: Dell Inc. Precision Tower 3620/0MWYPT,
BIOS 2.13.1 06/14/2019
[13670.913476] Call Trace:
[13670.913482]  <TASK>
[13670.913486]  dump_stack_lvl+0x48/0x70
[13670.913492]  dump_stack+0x10/0x20
[13670.913495]  warn_alloc+0x138/0x1b0
[13670.913501]  __alloc_pages+0x116b/0x1200
[13670.913504]  ? blk_mq_sched_try_merge+0x1be/0x1d0
[13670.913511]  ? policy_nodemask+0x10f/0x150
[13670.913516]  alloc_pages_mpol+0x91/0x220
[13670.913518]  ? filemap_get_entry+0xf3/0x180
[13670.913523]  alloc_pages+0x5e/0xd0
[13670.913525]  folio_alloc+0x18/0x50
[13670.913527]  filemap_alloc_folio+0xf5/0x100
[13670.913530]  __filemap_get_folio+0x112/0x2f0
[13670.913534]  __getblk_slow+0xba/0x250
[13670.913539]  __breadahead+0x8f/0xc0
[13670.913544]  exfat_get_dentry+0x155/0x1d0 [exfat]
[13670.913553]  ? _raw_spin_lock_irqsave+0x28/0x60
[13670.913557]  exfat_find_dir_entry+0x17b/0x740 [exfat]
[13670.913566]  ? __exfat_resolve_path+0x102/0x160 [exfat]
[13670.913574]  exfat_find+0xdd/0x340 [exfat]
[13670.913580]  ? update_load_avg+0x82/0x7d0
[13670.913584]  ? update_cfs_group+0xab/0xc0
[13670.913586]  ? psi_group_change+0x237/0x4e0
[13670.913590]  ? _raw_spin_unlock+0x19/0x40
[13670.913593]  ? raw_spin_rq_unlock+0x10/0x40
[13670.913598]  ? debug_smp_processor_id+0x17/0x20
[13670.913600]  ? mod_objcg_state+0xd0/0x350
[13670.913604]  ? memcg_slab_post_alloc_hook+0x195/0x230
[13670.913609]  ? kmem_cache_alloc_lru+0x1ca/0x3e0
[13670.913611]  ? __d_alloc+0x2e/0x200
[13670.913616]  exfat_lookup+0x55/0x200 [exfat]
[13670.913622]  ? preempt_count_add+0x54/0xc0
[13670.913626]  ? d_alloc_parallel+0x318/0x470
[13670.913628]  ? generic_permission+0x38/0x220
[13670.913632]  path_openat+0x588/0x1150
[13670.913637]  do_filp_open+0xb2/0x160
[13670.913643]  ? _raw_spin_unlock+0x19/0x40
[13670.913645]  ? alloc_fd+0xa9/0x190
[13670.913649]  do_sys_openat2+0x9b/0xd0
[13670.913653]  __x64_sys_creat+0x4b/0x70
[13670.913656]  do_syscall_64+0x5c/0xe0
[13670.913659]  ? __fput+0x16c/0x2f0
[13670.913661]  ? kmem_cache_free+0x190/0x3a0
[13670.913663]  ? percpu_counter_add_batch+0x35/0xb0
[13670.913666]  ? debug_smp_processor_id+0x17/0x20
[13670.913668]  ? fpregs_assert_state_consistent+0x2a/0x50
[13670.913672]  ? exit_to_user_mode_prepare+0x49/0x1a0
[13670.913675]  ? syscall_exit_to_user_mode+0x34/0x50
[13670.913677]  ? do_syscall_64+0x6b/0xe0
[13670.913679]  ? do_syscall_64+0x6b/0xe0
[13670.913682]  ? __x64_sys_write+0x19/0x20
[13670.913684]  ? do_syscall_64+0x6b/0xe0
[13670.913686]  ? sysvec_apic_timer_interrupt+0x66/0xd0
[13670.913689]  entry_SYSCALL_64_after_hwframe+0x6e/0x76

>
> Changes for v6:
>   - Fix build warning of printk() on 32-bit system.
>   - Fix read/write judgment in exfat_direct_IO().
>   - Remove update ei->valid_size in exfat_file_zeroed_range().
>
> Changes for v5:
>   - do not call exfat_map_new_buffer() if iblock + max_blocks < valid_blks.
>   - Reorganized the logic of exfat_get_block(), both writing and reading
> use
>     block index judgment.
>   - Remove unnecessary code moves.
>   - Reduce sync in exfat_file_write_iter()
>
> 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 zero the extended part
>
> Yuezhang Mo (2):
>   exfat: change to get file size from DataLength
>   exfat: do not zero the extended part
>
>  fs/exfat/exfat_fs.h |   2 +
>  fs/exfat/file.c     | 193 +++++++++++++++++++++++++++++++++++++++-----
>  fs/exfat/inode.c    | 136 +++++++++++++++++++++++++++----
>  fs/exfat/namei.c    |   6 ++
>  4 files changed, 299 insertions(+), 38 deletions(-)
>
> --
> 2.25.1
>

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

* RE: [PATCH v6 0/2] exfat: get file size from DataLength
       [not found]       ` <PUZPR04MB63160A6FD8E7EF04E4342B1B818EA@PUZPR04MB6316.apcprd04.prod.outlook.com>
@ 2023-12-12 10:29         ` Yuezhang.Mo
  2023-12-13  4:14           ` Namjae Jeon
  0 siblings, 1 reply; 16+ messages in thread
From: Yuezhang.Mo @ 2023-12-12 10:29 UTC (permalink / raw
  To: linkinjeon@kernel.org
  Cc: sj1557.seo@samsung.com, linux-fsdevel@vger.kernel.org,
	Andy.Wu@sony.com, Wataru.Aoyama@sony.com, cpgs@samsung.com,
	Dan Carpenter

> Have you ever run xfstests against v6 version ?

Yes. 

> The page allocation failure problem in my test happen below.
> What is your test result?

There is no the page allocation failure problem in my test.
I skipped the 6 test cases, except for these 6 test cases, all other test cases passed.

- generic/251
- generic/455
- generic/482
- generic/604
- generic/633
- generic/730

What are the chances of this problem happening? every time when running generic/476?



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

* Re: [PATCH v6 0/2] exfat: get file size from DataLength
  2023-12-12 10:29         ` Yuezhang.Mo
@ 2023-12-13  4:14           ` Namjae Jeon
  0 siblings, 0 replies; 16+ messages in thread
From: Namjae Jeon @ 2023-12-13  4:14 UTC (permalink / raw
  To: Yuezhang.Mo@sony.com
  Cc: sj1557.seo@samsung.com, linux-fsdevel@vger.kernel.org,
	Andy.Wu@sony.com, Wataru.Aoyama@sony.com, cpgs@samsung.com,
	Dan Carpenter

2023-12-12 19:29 GMT+09:00, Yuezhang.Mo@sony.com <Yuezhang.Mo@sony.com>:
>> Have you ever run xfstests against v6 version ?
>
> Yes.
>
>> The page allocation failure problem in my test happen below.
>> What is your test result?
>
> There is no the page allocation failure problem in my test.
> I skipped the 6 test cases, except for these 6 test cases, all other test
> cases passed.
>
> - generic/251
> - generic/455
> - generic/482
> - generic/604
> - generic/633
> - generic/730
>
> What are the chances of this problem happening? every time when running
> generic/476?
Hm.. I have checked It can happen regardless of your patches. It is
difficult to say how often this occurs. Because the generic/476 test
takes very long time. so I'll look into it further.
And I will apply your patches to #dev with Sungjong Reviewed-by tag.

Thanks!
>
>
>

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

end of thread, other threads:[~2023-12-13  4:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-02  9:58 [PATCH v4 0/2] exfat: get file size from DataLength Yuezhang.Mo
2023-11-30  3:09 ` [PATCH v5 " Yuezhang.Mo
2023-12-05 10:16   ` [PATCH v6 " Yuezhang.Mo
2023-12-12  4:12     ` Namjae Jeon
     [not found]       ` <PUZPR04MB63160A6FD8E7EF04E4342B1B818EA@PUZPR04MB6316.apcprd04.prod.outlook.com>
2023-12-12 10:29         ` Yuezhang.Mo
2023-12-13  4:14           ` Namjae Jeon
2023-12-05 10:16   ` [PATCH v6 1/2] exfat: change to " Yuezhang.Mo
2023-12-05 10:16   ` [PATCH v6 2/2] exfat: do not zero the extended part Yuezhang.Mo
2023-11-30  3:09 ` [PATCH v5 1/2] exfat: change to get file size from DataLength Yuezhang.Mo
2023-11-30 17:10   ` kernel test robot
2023-11-30 18:04   ` kernel test robot
2023-12-01  8:29   ` Dan Carpenter
2023-12-05  2:11     ` Sungjong Seo
2023-12-05  3:30   ` Namjae Jeon
     [not found]     ` <PUZPR04MB6316B8BAC361A5B2A70FD5098185A@PUZPR04MB6316.apcprd04.prod.outlook.com>
2023-12-05  5:29       ` Yuezhang.Mo
2023-11-30  3:09 ` [PATCH v5 2/2] exfat: do not zero the extended part Yuezhang.Mo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).