linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 01/11] exfat: add __exfat_get_dentry_set() helper
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-22  5:05   ` Namjae Jeon
  2023-12-08 11:23 ` [PATCH v1 02/11] exfat: add exfat_get_empty_dentry_set() helper yuezhang.mo
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

Since exfat_get_dentry_set() invokes the validate functions of
exfat_validate_entry(), it only supports getting a directory
entry set of an existing file, doesn't support getting an empty
entry set.

To remove the limitation, add this helper.

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/dir.c | 61 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 9f9295847a4e..f33e40aad276 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -775,7 +775,6 @@ struct exfat_dentry *exfat_get_dentry(struct super_block *sb,
 }
 
 enum exfat_validate_dentry_mode {
-	ES_MODE_STARTED,
 	ES_MODE_GET_FILE_ENTRY,
 	ES_MODE_GET_STRM_ENTRY,
 	ES_MODE_GET_NAME_ENTRY,
@@ -790,11 +789,6 @@ static bool exfat_validate_entry(unsigned int type,
 		return false;
 
 	switch (*mode) {
-	case ES_MODE_STARTED:
-		if  (type != TYPE_FILE && type != TYPE_DIR)
-			return false;
-		*mode = ES_MODE_GET_FILE_ENTRY;
-		break;
 	case ES_MODE_GET_FILE_ENTRY:
 		if (type != TYPE_STREAM)
 			return false;
@@ -834,7 +828,7 @@ struct exfat_dentry *exfat_get_dentry_cached(
 }
 
 /*
- * Returns a set of dentries for a file or dir.
+ * Returns a set of dentries.
  *
  * Note It provides a direct pointer to bh->data via exfat_get_dentry_cached().
  * User should call exfat_get_dentry_set() after setting 'modified' to apply
@@ -842,22 +836,24 @@ struct exfat_dentry *exfat_get_dentry_cached(
  *
  * in:
  *   sb+p_dir+entry: indicates a file/dir
- *   type:  specifies how many dentries should be included.
+ *   num_entries: specifies how many dentries should be included.
+ *                It will be set to es->num_entries if it is not 0.
+ *                If num_entries is 0, es->num_entries will be obtained
+ *                from the first dentry.
+ * out:
+ *   es: pointer of entry set on success.
  * return:
- *   pointer of entry set on success,
- *   NULL on failure.
+ *   0 on success
+ *   < 0 on failure
  */
-int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
+static int __exfat_get_dentry_set(struct exfat_entry_set_cache *es,
 		struct super_block *sb, struct exfat_chain *p_dir, int entry,
-		unsigned int type)
+		unsigned int num_entries)
 {
 	int ret, i, num_bh;
 	unsigned int off;
 	sector_t sec;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
-	struct exfat_dentry *ep;
-	int num_entries;
-	enum exfat_validate_dentry_mode mode = ES_MODE_STARTED;
 	struct buffer_head *bh;
 
 	if (p_dir->dir == DIR_DELETED) {
@@ -880,12 +876,16 @@ int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
 		return -EIO;
 	es->bh[es->num_bh++] = bh;
 
-	ep = exfat_get_dentry_cached(es, ES_IDX_FILE);
-	if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
-		goto put_es;
+	if (num_entries == ES_ALL_ENTRIES) {
+		struct exfat_dentry *ep;
+
+		ep = exfat_get_dentry_cached(es, ES_IDX_FILE);
+		if (ep->type == EXFAT_FILE)
+			num_entries = ep->dentry.file.num_ext + 1;
+		else
+			goto put_es;
+	}
 
-	num_entries = type == ES_ALL_ENTRIES ?
-		ep->dentry.file.num_ext + 1 : type;
 	es->num_entries = num_entries;
 
 	num_bh = EXFAT_B_TO_BLK_ROUND_UP(off + num_entries * DENTRY_SIZE, sb);
@@ -918,8 +918,27 @@ int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
 		es->bh[es->num_bh++] = bh;
 	}
 
+	return 0;
+
+put_es:
+	exfat_put_dentry_set(es, false);
+	return -EIO;
+}
+
+int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
+		struct super_block *sb, struct exfat_chain *p_dir,
+		int entry, unsigned int type)
+{
+	int ret, i;
+	struct exfat_dentry *ep;
+	enum exfat_validate_dentry_mode mode = ES_MODE_GET_FILE_ENTRY;
+
+	ret = __exfat_get_dentry_set(es, sb, p_dir, entry, type);
+	if (ret < 0)
+		return ret;
+
 	/* validate cached dentries */
-	for (i = ES_IDX_STREAM; i < num_entries; i++) {
+	for (i = ES_IDX_STREAM; i < es->num_entries; i++) {
 		ep = exfat_get_dentry_cached(es, i);
 		if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
 			goto put_es;
-- 
2.25.1


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

* [PATCH v1 02/11] exfat: add exfat_get_empty_dentry_set() helper
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
  2023-12-08 11:23 ` [PATCH v1 01/11] exfat: add __exfat_get_dentry_set() helper yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-08 11:23 ` [PATCH v1 03/11] exfat: covert exfat_find_empty_entry() to use dentry cache yuezhang.mo
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

This helper is used to lookup empty dentry set. If there are
enough empty dentries at the input location, this helper will
return the number of dentries that need to be skipped for the
next lookup.

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/dir.c      | 77 +++++++++++++++++++++++++++++++++++++++++++++
 fs/exfat/exfat_fs.h |  3 ++
 2 files changed, 80 insertions(+)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index f33e40aad276..bb23585c6e7c 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -950,6 +950,83 @@ int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
 	return -EIO;
 }
 
+static int exfat_validate_empty_dentry_set(struct exfat_entry_set_cache *es)
+{
+	struct exfat_dentry *ep;
+	struct buffer_head *bh;
+	int i, off;
+	bool unused_hit = false;
+
+	for (i = 0; i < es->num_entries; i++) {
+		ep = exfat_get_dentry_cached(es, i);
+		if (ep->type == EXFAT_UNUSED)
+			unused_hit = true;
+		else if (IS_EXFAT_DELETED(ep->type)) {
+			if (unused_hit)
+				goto out;
+		} else {
+			if (unused_hit)
+				goto out;
+
+			i++;
+			goto count_skip_entries;
+		}
+	}
+
+	return 0;
+
+out:
+	off = es->start_off + (i << DENTRY_SIZE_BITS);
+	bh = es->bh[EXFAT_B_TO_BLK(off, es->sb)];
+
+	exfat_fs_error(es->sb,
+		"in sector %lld, dentry %d should be unused, but 0x%x",
+		bh->b_blocknr, off >> DENTRY_SIZE_BITS, ep->type);
+
+	return -EIO;
+
+count_skip_entries:
+	es->num_entries = EXFAT_B_TO_DEN(EXFAT_BLK_TO_B(es->num_bh, es->sb) - es->start_off);
+	for (; i < es->num_entries; i++) {
+		ep = exfat_get_dentry_cached(es, i);
+		if (IS_EXFAT_DELETED(ep->type))
+			break;
+	}
+
+	return i;
+}
+
+/*
+ * Get an empty dentry set.
+ *
+ * in:
+ *   sb+p_dir+entry: indicates the empty dentry location
+ *   num_entries: specifies how many empty dentries should be included.
+ * out:
+ *   es: pointer of empty dentry set on success.
+ * return:
+ *   0   : on success
+ *   >= 0: the dentries are not empty, the return value is the number of
+ *         dentries to be skipped for the next lookup.
+ *   <0  : on failure
+ */
+int exfat_get_empty_dentry_set(struct exfat_entry_set_cache *es,
+		struct super_block *sb, struct exfat_chain *p_dir,
+		int entry, unsigned int num_entries)
+{
+	int ret;
+
+	ret = __exfat_get_dentry_set(es, sb, p_dir, entry, num_entries);
+	if (ret < 0)
+		return ret;
+
+	ret = exfat_validate_empty_dentry_set(es);
+	if (ret)
+		exfat_put_dentry_set(es, false);
+
+	return ret;
+}
+
 static inline void exfat_reset_empty_hint(struct exfat_hint_femp *hint_femp)
 {
 	hint_femp->eidx = EXFAT_HINT_NONE;
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index e3b1f8e022df..6dc76b3f4945 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -502,6 +502,9 @@ struct exfat_dentry *exfat_get_dentry_cached(struct exfat_entry_set_cache *es,
 int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
 		struct super_block *sb, struct exfat_chain *p_dir, int entry,
 		unsigned int type);
+int exfat_get_empty_dentry_set(struct exfat_entry_set_cache *es,
+		struct super_block *sb, struct exfat_chain *p_dir, int entry,
+		unsigned int num_entries);
 int exfat_put_dentry_set(struct exfat_entry_set_cache *es, int sync);
 int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir);
 
-- 
2.25.1


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

* [PATCH v1 03/11] exfat: covert exfat_find_empty_entry() to use dentry cache
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
  2023-12-08 11:23 ` [PATCH v1 01/11] exfat: add __exfat_get_dentry_set() helper yuezhang.mo
  2023-12-08 11:23 ` [PATCH v1 02/11] exfat: add exfat_get_empty_dentry_set() helper yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-08 11:23 ` [PATCH v1 04/11] exfat: covert exfat_add_entry() " yuezhang.mo
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

Before this conversion, each dentry traversed needs to be read
from the storage device or page cache. There are at least 16
dentries in a sector. This will result in frequent page cache
searches.

After this conversion, if all directory entries in a sector are
used, the sector only needs to be read once.

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/namei.c | 111 ++++++++++++++++++++---------------------------
 1 file changed, 47 insertions(+), 64 deletions(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 9c549fd11fc8..1f662296f2fa 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -204,21 +204,16 @@ const struct dentry_operations exfat_utf8_dentry_ops = {
 	.d_compare	= exfat_utf8_d_cmp,
 };
 
-/* used only in search empty_slot() */
-#define CNT_UNUSED_NOHIT        (-1)
-#define CNT_UNUSED_HIT          (-2)
 /* search EMPTY CONTINUOUS "num_entries" entries */
 static int exfat_search_empty_slot(struct super_block *sb,
 		struct exfat_hint_femp *hint_femp, struct exfat_chain *p_dir,
-		int num_entries)
+		int num_entries, struct exfat_entry_set_cache *es)
 {
-	int i, dentry, num_empty = 0;
+	int i, dentry, ret;
 	int dentries_per_clu;
-	unsigned int type;
 	struct exfat_chain clu;
-	struct exfat_dentry *ep;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
-	struct buffer_head *bh;
+	int total_entries = EXFAT_CLU_TO_DEN(p_dir->size, sbi);
 
 	dentries_per_clu = sbi->dentries_per_clu;
 
@@ -231,7 +226,7 @@ static int exfat_search_empty_slot(struct super_block *sb,
 		 * Otherwise, and if "dentry + hint_famp->count" is also equal
 		 * to "p_dir->size * dentries_per_clu", it means ENOSPC.
 		 */
-		if (dentry + hint_femp->count == p_dir->size * dentries_per_clu &&
+		if (dentry + hint_femp->count == total_entries &&
 		    num_entries > hint_femp->count)
 			return -ENOSPC;
 
@@ -242,69 +237,41 @@ static int exfat_search_empty_slot(struct super_block *sb,
 		dentry = 0;
 	}
 
-	while (clu.dir != EXFAT_EOF_CLUSTER) {
+	while (dentry + num_entries < total_entries &&
+	       clu.dir != EXFAT_EOF_CLUSTER) {
 		i = dentry & (dentries_per_clu - 1);
 
-		for (; i < dentries_per_clu; i++, dentry++) {
-			ep = exfat_get_dentry(sb, &clu, i, &bh);
-			if (!ep)
-				return -EIO;
-			type = exfat_get_entry_type(ep);
-			brelse(bh);
-
-			if (type == TYPE_UNUSED || type == TYPE_DELETED) {
-				num_empty++;
-				if (hint_femp->eidx == EXFAT_HINT_NONE) {
-					hint_femp->eidx = dentry;
-					hint_femp->count = CNT_UNUSED_NOHIT;
-					exfat_chain_set(&hint_femp->cur,
-						clu.dir, clu.size, clu.flags);
-				}
-
-				if (type == TYPE_UNUSED &&
-				    hint_femp->count != CNT_UNUSED_HIT)
-					hint_femp->count = CNT_UNUSED_HIT;
+		ret = exfat_get_empty_dentry_set(es, sb, &clu, i, num_entries);
+		if (ret < 0)
+			return ret;
+		else if (ret == 0)
+			return dentry;
+
+		dentry += ret;
+		i += ret;
+
+		while (i >= dentries_per_clu) {
+			if (clu.flags == ALLOC_NO_FAT_CHAIN) {
+				if (--clu.size > 0)
+					clu.dir++;
+				else
+					clu.dir = EXFAT_EOF_CLUSTER;
 			} else {
-				if (hint_femp->eidx != EXFAT_HINT_NONE &&
-				    hint_femp->count == CNT_UNUSED_HIT) {
-					/* unused empty group means
-					 * an empty group which includes
-					 * unused dentry
-					 */
-					exfat_fs_error(sb,
-						"found bogus dentry(%d) beyond unused empty group(%d) (start_clu : %u, cur_clu : %u)",
-						dentry, hint_femp->eidx,
-						p_dir->dir, clu.dir);
+				if (exfat_get_next_cluster(sb, &clu.dir))
 					return -EIO;
-				}
-
-				num_empty = 0;
-				hint_femp->eidx = EXFAT_HINT_NONE;
 			}
 
-			if (num_empty >= num_entries) {
-				/* found and invalidate hint_femp */
-				hint_femp->eidx = EXFAT_HINT_NONE;
-				return (dentry - (num_entries - 1));
-			}
-		}
-
-		if (clu.flags == ALLOC_NO_FAT_CHAIN) {
-			if (--clu.size > 0)
-				clu.dir++;
-			else
-				clu.dir = EXFAT_EOF_CLUSTER;
-		} else {
-			if (exfat_get_next_cluster(sb, &clu.dir))
-				return -EIO;
+			i -= dentries_per_clu;
 		}
 	}
 
-	hint_femp->eidx = p_dir->size * dentries_per_clu - num_empty;
-	hint_femp->count = num_empty;
-	if (num_empty == 0)
+	hint_femp->eidx = dentry;
+	hint_femp->count = 0;
+	if (dentry == total_entries || clu.dir == EXFAT_EOF_CLUSTER)
 		exfat_chain_set(&hint_femp->cur, EXFAT_EOF_CLUSTER, 0,
 				clu.flags);
+	else
+		hint_femp->cur = clu;
 
 	return -ENOSPC;
 }
@@ -324,8 +291,9 @@ static int exfat_check_max_dentries(struct inode *inode)
 /* find empty directory entry.
  * if there isn't any empty slot, expand cluster chain.
  */
-static int exfat_find_empty_entry(struct inode *inode,
-		struct exfat_chain *p_dir, int num_entries)
+static int __exfat_find_empty_entry(struct inode *inode,
+		struct exfat_chain *p_dir, int num_entries,
+		struct exfat_entry_set_cache *es)
 {
 	int dentry;
 	unsigned int ret, last_clu;
@@ -344,7 +312,7 @@ static int exfat_find_empty_entry(struct inode *inode,
 	}
 
 	while ((dentry = exfat_search_empty_slot(sb, &hint_femp, p_dir,
-					num_entries)) < 0) {
+					num_entries, es)) < 0) {
 		if (dentry == -EIO)
 			break;
 
@@ -414,6 +382,21 @@ static int exfat_find_empty_entry(struct inode *inode,
 	return dentry;
 }
 
+static int exfat_find_empty_entry(struct inode *inode,
+		struct exfat_chain *p_dir, int num_entries)
+{
+	int entry;
+	struct exfat_entry_set_cache es;
+
+	entry = __exfat_find_empty_entry(inode, p_dir, num_entries, &es);
+	if (entry < 0)
+		return entry;
+
+	exfat_put_dentry_set(&es, false);
+
+	return entry;
+}
+
 /*
  * Name Resolution Functions :
  * Zero if it was successful; otherwise nonzero.
-- 
2.25.1


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

* [PATCH v1 04/11] exfat: covert exfat_add_entry() to use dentry cache
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
                   ` (2 preceding siblings ...)
  2023-12-08 11:23 ` [PATCH v1 03/11] exfat: covert exfat_find_empty_entry() to use dentry cache yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-08 11:23 ` [PATCH v1 05/11] exfat: covert exfat_remove_entries() " yuezhang.mo
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

After this conversion, if "dirsync" or "sync" is enabled, the
number of synchronized dentries in exfat_add_entry() will change
from 2 to 1.

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/dir.c      | 37 +++++++++----------------------------
 fs/exfat/exfat_fs.h |  6 +++---
 fs/exfat/namei.c    | 12 ++++++++++--
 3 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index bb23585c6e7c..a7eda14a57ac 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -448,53 +448,34 @@ static void exfat_init_name_entry(struct exfat_dentry *ep,
 	}
 }
 
-int exfat_init_dir_entry(struct inode *inode, struct exfat_chain *p_dir,
-		int entry, unsigned int type, unsigned int start_clu,
-		unsigned long long size)
+void exfat_init_dir_entry(struct exfat_entry_set_cache *es,
+		unsigned int type, unsigned int start_clu,
+		unsigned long long size, struct timespec64 *ts)
 {
-	struct super_block *sb = inode->i_sb;
+	struct super_block *sb = es->sb;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
-	struct timespec64 ts = current_time(inode);
 	struct exfat_dentry *ep;
-	struct buffer_head *bh;
-
-	/*
-	 * We cannot use exfat_get_dentry_set here because file ep is not
-	 * initialized yet.
-	 */
-	ep = exfat_get_dentry(sb, p_dir, entry, &bh);
-	if (!ep)
-		return -EIO;
 
+	ep = exfat_get_dentry_cached(es, ES_IDX_FILE);
 	exfat_set_entry_type(ep, type);
-	exfat_set_entry_time(sbi, &ts,
+	exfat_set_entry_time(sbi, ts,
 			&ep->dentry.file.create_tz,
 			&ep->dentry.file.create_time,
 			&ep->dentry.file.create_date,
 			&ep->dentry.file.create_time_cs);
-	exfat_set_entry_time(sbi, &ts,
+	exfat_set_entry_time(sbi, ts,
 			&ep->dentry.file.modify_tz,
 			&ep->dentry.file.modify_time,
 			&ep->dentry.file.modify_date,
 			&ep->dentry.file.modify_time_cs);
-	exfat_set_entry_time(sbi, &ts,
+	exfat_set_entry_time(sbi, ts,
 			&ep->dentry.file.access_tz,
 			&ep->dentry.file.access_time,
 			&ep->dentry.file.access_date,
 			NULL);
 
-	exfat_update_bh(bh, IS_DIRSYNC(inode));
-	brelse(bh);
-
-	ep = exfat_get_dentry(sb, p_dir, entry + 1, &bh);
-	if (!ep)
-		return -EIO;
-
+	ep = exfat_get_dentry_cached(es, ES_IDX_STREAM);
 	exfat_init_stream_entry(ep, start_clu, size);
-	exfat_update_bh(bh, IS_DIRSYNC(inode));
-	brelse(bh);
-
-	return 0;
 }
 
 int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 6dc76b3f4945..0897584d1473 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -480,9 +480,9 @@ int exfat_get_cluster(struct inode *inode, unsigned int cluster,
 extern const struct inode_operations exfat_dir_inode_operations;
 extern const struct file_operations exfat_dir_operations;
 unsigned int exfat_get_entry_type(struct exfat_dentry *p_entry);
-int exfat_init_dir_entry(struct inode *inode, struct exfat_chain *p_dir,
-		int entry, unsigned int type, unsigned int start_clu,
-		unsigned long long size);
+void exfat_init_dir_entry(struct exfat_entry_set_cache *es,
+		unsigned int type, unsigned int start_clu,
+		unsigned long long size, struct timespec64 *ts);
 int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
 		int entry, int num_entries, struct exfat_uni_name *p_uniname);
 int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 1f662296f2fa..423cd6d505ab 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -482,6 +482,8 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	struct exfat_uni_name uniname;
 	struct exfat_chain clu;
+	struct timespec64 ts = current_time(inode);
+	struct exfat_entry_set_cache es;
 	int clu_size = 0;
 	unsigned int start_clu = EXFAT_FREE_CLUSTER;
 
@@ -514,8 +516,14 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 	/* fill the dos name directory entry information of the created file.
 	 * the first cluster is not determined yet. (0)
 	 */
-	ret = exfat_init_dir_entry(inode, p_dir, dentry, type,
-		start_clu, clu_size);
+
+	ret = exfat_get_empty_dentry_set(&es, sb, p_dir, dentry, num_entries);
+	if (ret)
+		goto out;
+
+	exfat_init_dir_entry(&es, type, start_clu, clu_size, &ts);
+
+	ret = exfat_put_dentry_set(&es, IS_DIRSYNC(inode));
 	if (ret)
 		goto out;
 
-- 
2.25.1


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

* [PATCH v1 05/11] exfat: covert exfat_remove_entries() to use dentry cache
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
                   ` (3 preceding siblings ...)
  2023-12-08 11:23 ` [PATCH v1 04/11] exfat: covert exfat_add_entry() " yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-22  5:07   ` Namjae Jeon
  2023-12-08 11:23 ` [PATCH v1 06/11] exfat: move free cluster out of exfat_init_ext_entry() yuezhang.mo
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

Before this conversion, in exfat_remove_entries(), to mark the
dentries in a dentry set as deleted, the sync times is equals
the dentry numbers if 'dirsync' or 'sync' is enabled.
That affects not only performance but also device life.

After this conversion, only needs to be synchronized once if
'dirsync' or 'sync' is enabled.

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/dir.c      |  17 ++--
 fs/exfat/exfat_fs.h |   4 +-
 fs/exfat/namei.c    | 184 ++++++++++++++++++++------------------------
 3 files changed, 90 insertions(+), 115 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index a7eda14a57ac..2a002b01f1dc 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -577,28 +577,23 @@ int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
 	return 0;
 }
 
-int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
-		int entry, int order, int num_entries)
+void exfat_remove_entries(struct inode *inode, struct exfat_entry_set_cache *es,
+		int order)
 {
-	struct super_block *sb = inode->i_sb;
 	int i;
 	struct exfat_dentry *ep;
-	struct buffer_head *bh;
 
-	for (i = order; i < num_entries; i++) {
-		ep = exfat_get_dentry(sb, p_dir, entry + i, &bh);
-		if (!ep)
-			return -EIO;
+	for (i = order; i < es->num_entries; i++) {
+		ep = exfat_get_dentry_cached(es, i);
 
 		if (exfat_get_entry_type(ep) & TYPE_BENIGN_SEC)
 			exfat_free_benign_secondary_clusters(inode, ep);
 
 		exfat_set_entry_type(ep, TYPE_DELETED);
-		exfat_update_bh(bh, IS_DIRSYNC(inode));
-		brelse(bh);
 	}
 
-	return 0;
+	if (order < es->num_entries)
+		es->modified = true;
 }
 
 void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 0897584d1473..46031e77f58b 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -485,8 +485,8 @@ void exfat_init_dir_entry(struct exfat_entry_set_cache *es,
 		unsigned long long size, struct timespec64 *ts);
 int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
 		int entry, int num_entries, struct exfat_uni_name *p_uniname);
-int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
-		int entry, int order, int num_entries);
+void exfat_remove_entries(struct inode *inode, struct exfat_entry_set_cache *es,
+		int order);
 int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
 		int entry);
 void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es);
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 423cd6d505ab..6a85b6707a7e 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -786,12 +786,11 @@ static struct dentry *exfat_lookup(struct inode *dir, struct dentry *dentry,
 static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 {
 	struct exfat_chain cdir;
-	struct exfat_dentry *ep;
 	struct super_block *sb = dir->i_sb;
 	struct inode *inode = dentry->d_inode;
 	struct exfat_inode_info *ei = EXFAT_I(inode);
-	struct buffer_head *bh;
-	int num_entries, entry, err = 0;
+	struct exfat_entry_set_cache es;
+	int entry, err = 0;
 
 	mutex_lock(&EXFAT_SB(sb)->s_lock);
 	exfat_chain_dup(&cdir, &ei->dir);
@@ -802,26 +801,20 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 		goto unlock;
 	}
 
-	ep = exfat_get_dentry(sb, &cdir, entry, &bh);
-	if (!ep) {
-		err = -EIO;
-		goto unlock;
-	}
-	num_entries = exfat_count_ext_entries(sb, &cdir, entry, ep);
-	if (num_entries < 0) {
+	err = exfat_get_dentry_set(&es, sb, &cdir, entry, ES_ALL_ENTRIES);
+	if (err) {
 		err = -EIO;
-		brelse(bh);
 		goto unlock;
 	}
-	num_entries++;
-	brelse(bh);
 
 	exfat_set_volume_dirty(sb);
+
 	/* update the directory entry */
-	if (exfat_remove_entries(dir, &cdir, entry, 0, num_entries)) {
-		err = -EIO;
+	exfat_remove_entries(inode, &es, ES_IDX_FILE);
+
+	err = exfat_put_dentry_set(&es, IS_DIRSYNC(inode));
+	if (err)
 		goto unlock;
-	}
 
 	/* This doesn't modify ei */
 	ei->dir.dir = DIR_DELETED;
@@ -937,13 +930,12 @@ static int exfat_check_dir_empty(struct super_block *sb,
 static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
-	struct exfat_dentry *ep;
 	struct exfat_chain cdir, clu_to_free;
 	struct super_block *sb = inode->i_sb;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	struct exfat_inode_info *ei = EXFAT_I(inode);
-	struct buffer_head *bh;
-	int num_entries, entry, err;
+	struct exfat_entry_set_cache es;
+	int entry, err;
 
 	mutex_lock(&EXFAT_SB(inode->i_sb)->s_lock);
 
@@ -967,27 +959,20 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
 		goto unlock;
 	}
 
-	ep = exfat_get_dentry(sb, &cdir, entry, &bh);
-	if (!ep) {
-		err = -EIO;
-		goto unlock;
-	}
-
-	num_entries = exfat_count_ext_entries(sb, &cdir, entry, ep);
-	if (num_entries < 0) {
+	err = exfat_get_dentry_set(&es, sb, &cdir, entry, ES_ALL_ENTRIES);
+	if (err) {
 		err = -EIO;
-		brelse(bh);
 		goto unlock;
 	}
-	num_entries++;
-	brelse(bh);
 
 	exfat_set_volume_dirty(sb);
-	err = exfat_remove_entries(dir, &cdir, entry, 0, num_entries);
-	if (err) {
-		exfat_err(sb, "failed to exfat_remove_entries : err(%d)", err);
+
+	exfat_remove_entries(inode, &es, ES_IDX_FILE);
+
+	err = exfat_put_dentry_set(&es, IS_DIRSYNC(dir));
+	if (err)
 		goto unlock;
-	}
+
 	ei->dir.dir = DIR_DELETED;
 
 	inode_inc_iversion(dir);
@@ -1013,36 +998,40 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 		int oldentry, struct exfat_uni_name *p_uniname,
 		struct exfat_inode_info *ei)
 {
-	int ret, num_old_entries, num_new_entries;
+	int ret, num_new_entries;
 	struct exfat_dentry *epold, *epnew;
 	struct super_block *sb = inode->i_sb;
-	struct buffer_head *new_bh, *old_bh;
+	struct buffer_head *new_bh;
+	struct exfat_entry_set_cache old_es;
 	int sync = IS_DIRSYNC(inode);
 
-	epold = exfat_get_dentry(sb, p_dir, oldentry, &old_bh);
-	if (!epold)
-		return -EIO;
-
-	num_old_entries = exfat_count_ext_entries(sb, p_dir, oldentry, epold);
-	if (num_old_entries < 0)
-		return -EIO;
-	num_old_entries++;
-
 	num_new_entries = exfat_calc_num_entries(p_uniname);
 	if (num_new_entries < 0)
 		return num_new_entries;
 
-	if (num_old_entries < num_new_entries) {
+	ret = exfat_get_dentry_set(&old_es, sb, p_dir, oldentry, ES_ALL_ENTRIES);
+	if (ret) {
+		ret = -EIO;
+		return ret;
+	}
+
+	epold = exfat_get_dentry_cached(&old_es, ES_IDX_FILE);
+
+	if (old_es.num_entries < num_new_entries) {
 		int newentry;
 
 		newentry =
 			exfat_find_empty_entry(inode, p_dir, num_new_entries);
-		if (newentry < 0)
-			return newentry; /* -EIO or -ENOSPC */
+		if (newentry < 0) {
+			ret = newentry; /* -EIO or -ENOSPC */
+			goto put_old_es;
+		}
 
 		epnew = exfat_get_dentry(sb, p_dir, newentry, &new_bh);
-		if (!epnew)
-			return -EIO;
+		if (!epnew) {
+			ret = -EIO;
+			goto put_old_es;
+		}
 
 		*epnew = *epold;
 		if (exfat_get_entry_type(epnew) == TYPE_FILE) {
@@ -1050,30 +1039,25 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 			ei->attr |= EXFAT_ATTR_ARCHIVE;
 		}
 		exfat_update_bh(new_bh, sync);
-		brelse(old_bh);
 		brelse(new_bh);
 
-		epold = exfat_get_dentry(sb, p_dir, oldentry + 1, &old_bh);
-		if (!epold)
-			return -EIO;
+		epold = exfat_get_dentry_cached(&old_es, ES_IDX_STREAM);
 		epnew = exfat_get_dentry(sb, p_dir, newentry + 1, &new_bh);
 		if (!epnew) {
-			brelse(old_bh);
-			return -EIO;
+			ret = -EIO;
+			goto put_old_es;
 		}
 
 		*epnew = *epold;
 		exfat_update_bh(new_bh, sync);
-		brelse(old_bh);
 		brelse(new_bh);
 
 		ret = exfat_init_ext_entry(inode, p_dir, newentry,
 			num_new_entries, p_uniname);
 		if (ret)
-			return ret;
+			goto put_old_es;
 
-		exfat_remove_entries(inode, p_dir, oldentry, 0,
-			num_old_entries);
+		exfat_remove_entries(inode, &old_es, ES_IDX_FILE);
 		ei->dir = *p_dir;
 		ei->entry = newentry;
 	} else {
@@ -1081,37 +1065,29 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 			epold->dentry.file.attr |= cpu_to_le16(EXFAT_ATTR_ARCHIVE);
 			ei->attr |= EXFAT_ATTR_ARCHIVE;
 		}
-		exfat_update_bh(old_bh, sync);
-		brelse(old_bh);
 		ret = exfat_init_ext_entry(inode, p_dir, oldentry,
 			num_new_entries, p_uniname);
 		if (ret)
-			return ret;
+			goto put_old_es;
 
-		exfat_remove_entries(inode, p_dir, oldentry, num_new_entries,
-			num_old_entries);
+		exfat_remove_entries(inode, &old_es, num_new_entries);
 	}
-	return 0;
+	return exfat_put_dentry_set(&old_es, sync);
+
+put_old_es:
+	exfat_put_dentry_set(&old_es, false);
+	return ret;
 }
 
 static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 		int oldentry, struct exfat_chain *p_newdir,
 		struct exfat_uni_name *p_uniname, struct exfat_inode_info *ei)
 {
-	int ret, newentry, num_new_entries, num_old_entries;
+	int ret, newentry, num_new_entries;
 	struct exfat_dentry *epmov, *epnew;
 	struct super_block *sb = inode->i_sb;
-	struct buffer_head *mov_bh, *new_bh;
-
-	epmov = exfat_get_dentry(sb, p_olddir, oldentry, &mov_bh);
-	if (!epmov)
-		return -EIO;
-
-	num_old_entries = exfat_count_ext_entries(sb, p_olddir, oldentry,
-		epmov);
-	if (num_old_entries < 0)
-		return -EIO;
-	num_old_entries++;
+	struct buffer_head *new_bh;
+	struct exfat_entry_set_cache mov_es;
 
 	num_new_entries = exfat_calc_num_entries(p_uniname);
 	if (num_new_entries < 0)
@@ -1121,31 +1097,35 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 	if (newentry < 0)
 		return newentry; /* -EIO or -ENOSPC */
 
-	epnew = exfat_get_dentry(sb, p_newdir, newentry, &new_bh);
-	if (!epnew)
+	ret = exfat_get_dentry_set(&mov_es, sb, p_olddir, oldentry,
+			ES_ALL_ENTRIES);
+	if (ret)
 		return -EIO;
 
+	epmov = exfat_get_dentry_cached(&mov_es, ES_IDX_FILE);
+	epnew = exfat_get_dentry(sb, p_newdir, newentry, &new_bh);
+	if (!epnew) {
+		ret = -EIO;
+		goto put_mov_es;
+	}
+
 	*epnew = *epmov;
 	if (exfat_get_entry_type(epnew) == TYPE_FILE) {
 		epnew->dentry.file.attr |= cpu_to_le16(EXFAT_ATTR_ARCHIVE);
 		ei->attr |= EXFAT_ATTR_ARCHIVE;
 	}
 	exfat_update_bh(new_bh, IS_DIRSYNC(inode));
-	brelse(mov_bh);
 	brelse(new_bh);
 
-	epmov = exfat_get_dentry(sb, p_olddir, oldentry + 1, &mov_bh);
-	if (!epmov)
-		return -EIO;
+	epmov = exfat_get_dentry_cached(&mov_es, ES_IDX_STREAM);
 	epnew = exfat_get_dentry(sb, p_newdir, newentry + 1, &new_bh);
 	if (!epnew) {
-		brelse(mov_bh);
-		return -EIO;
+		ret = -EIO;
+		goto put_mov_es;
 	}
 
 	*epnew = *epmov;
 	exfat_update_bh(new_bh, IS_DIRSYNC(inode));
-	brelse(mov_bh);
 	brelse(new_bh);
 
 	ret = exfat_init_ext_entry(inode, p_newdir, newentry, num_new_entries,
@@ -1153,13 +1133,18 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 	if (ret)
 		return ret;
 
-	exfat_remove_entries(inode, p_olddir, oldentry, 0, num_old_entries);
+	exfat_remove_entries(inode, &mov_es, ES_IDX_FILE);
 
 	exfat_chain_set(&ei->dir, p_newdir->dir, p_newdir->size,
 		p_newdir->flags);
 
 	ei->entry = newentry;
-	return 0;
+	return exfat_put_dentry_set(&mov_es, IS_DIRSYNC(inode));
+
+put_mov_es:
+	exfat_put_dentry_set(&mov_es, false);
+
+	return ret;
 }
 
 /* rename or move a old file into a new file */
@@ -1177,7 +1162,6 @@ static int __exfat_rename(struct inode *old_parent_inode,
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	const unsigned char *new_path = new_dentry->d_name.name;
 	struct inode *new_inode = new_dentry->d_inode;
-	int num_entries;
 	struct exfat_inode_info *new_ei = NULL;
 	unsigned int new_entry_type = TYPE_UNUSED;
 	int new_entry = 0;
@@ -1248,25 +1232,21 @@ static int __exfat_rename(struct inode *old_parent_inode,
 				&newdir, &uni_name, ei);
 
 	if (!ret && new_inode) {
+		struct exfat_entry_set_cache es;
+
 		/* delete entries of new_dir */
-		ep = exfat_get_dentry(sb, p_dir, new_entry, &new_bh);
-		if (!ep) {
+		ret = exfat_get_dentry_set(&es, sb, p_dir, new_entry,
+				ES_ALL_ENTRIES);
+		if (ret) {
 			ret = -EIO;
 			goto del_out;
 		}
 
-		num_entries = exfat_count_ext_entries(sb, p_dir, new_entry, ep);
-		if (num_entries < 0) {
-			ret = -EIO;
-			goto del_out;
-		}
-		brelse(new_bh);
+		exfat_remove_entries(new_inode, &es, ES_IDX_FILE);
 
-		if (exfat_remove_entries(new_inode, p_dir, new_entry, 0,
-				num_entries + 1)) {
-			ret = -EIO;
+		ret = exfat_put_dentry_set(&es, IS_DIRSYNC(new_inode));
+		if (ret)
 			goto del_out;
-		}
 
 		/* Free the clusters if new_inode is a dir(as if exfat_rmdir) */
 		if (new_entry_type == TYPE_DIR &&
-- 
2.25.1


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

* [PATCH v1 06/11] exfat: move free cluster out of exfat_init_ext_entry()
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
                   ` (4 preceding siblings ...)
  2023-12-08 11:23 ` [PATCH v1 05/11] exfat: covert exfat_remove_entries() " yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-08 11:23 ` [PATCH v1 07/11] exfat: covert exfat_init_ext_entry() to use dentry cache yuezhang.mo
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

exfat_init_ext_entry() is an init function, it's a bit strange
to free cluster in it. And the argument 'inode' will be removed
from exfat_init_ext_entry(). So this commit changes to free the
cluster in exfat_remove_entries().

Code refinement, no functional changes.

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/dir.c   | 3 ---
 fs/exfat/namei.c | 5 +++--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 2a002b01f1dc..8965bb2c99ae 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -564,9 +564,6 @@ int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
 		if (!ep)
 			return -EIO;
 
-		if (exfat_get_entry_type(ep) & TYPE_BENIGN_SEC)
-			exfat_free_benign_secondary_clusters(inode, ep);
-
 		exfat_init_name_entry(ep, uniname);
 		exfat_update_bh(bh, sync);
 		brelse(bh);
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 6a85b6707a7e..9a0d8f2deea6 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -1065,12 +1065,13 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 			epold->dentry.file.attr |= cpu_to_le16(EXFAT_ATTR_ARCHIVE);
 			ei->attr |= EXFAT_ATTR_ARCHIVE;
 		}
+
+		exfat_remove_entries(inode, &old_es, ES_IDX_FIRST_FILENAME + 1);
+
 		ret = exfat_init_ext_entry(inode, p_dir, oldentry,
 			num_new_entries, p_uniname);
 		if (ret)
 			goto put_old_es;
-
-		exfat_remove_entries(inode, &old_es, num_new_entries);
 	}
 	return exfat_put_dentry_set(&old_es, sync);
 
-- 
2.25.1


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

* [PATCH v1 07/11] exfat: covert exfat_init_ext_entry() to use dentry cache
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
                   ` (5 preceding siblings ...)
  2023-12-08 11:23 ` [PATCH v1 06/11] exfat: move free cluster out of exfat_init_ext_entry() yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-08 11:23 ` [PATCH v1 08/11] exfat: remove __exfat_find_empty_entry() yuezhang.mo
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

Before this conversion, in exfat_init_ext_entry(), to init
the dentries in a dentry set, the sync times is equals the
dentry number if 'dirsync' or 'sync' is enabled.
That affects not only performance but also device life.

After this conversion, only needs to be synchronized once if
'dirsync' or 'sync' is enabled.

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/dir.c      | 33 +++++---------------
 fs/exfat/exfat_fs.h |  4 +--
 fs/exfat/namei.c    | 73 +++++++++++++++------------------------------
 3 files changed, 33 insertions(+), 77 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 8965bb2c99ae..94cedc145291 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -532,46 +532,27 @@ static void exfat_free_benign_secondary_clusters(struct inode *inode,
 	exfat_free_cluster(inode, &dir);
 }
 
-int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
-		int entry, int num_entries, struct exfat_uni_name *p_uniname)
+void exfat_init_ext_entry(struct exfat_entry_set_cache *es, int num_entries,
+		struct exfat_uni_name *p_uniname)
 {
-	struct super_block *sb = inode->i_sb;
 	int i;
 	unsigned short *uniname = p_uniname->name;
 	struct exfat_dentry *ep;
-	struct buffer_head *bh;
-	int sync = IS_DIRSYNC(inode);
-
-	ep = exfat_get_dentry(sb, p_dir, entry, &bh);
-	if (!ep)
-		return -EIO;
 
+	ep = exfat_get_dentry_cached(es, ES_IDX_FILE);
 	ep->dentry.file.num_ext = (unsigned char)(num_entries - 1);
-	exfat_update_bh(bh, sync);
-	brelse(bh);
-
-	ep = exfat_get_dentry(sb, p_dir, entry + 1, &bh);
-	if (!ep)
-		return -EIO;
 
+	ep = exfat_get_dentry_cached(es, ES_IDX_STREAM);
 	ep->dentry.stream.name_len = p_uniname->name_len;
 	ep->dentry.stream.name_hash = cpu_to_le16(p_uniname->name_hash);
-	exfat_update_bh(bh, sync);
-	brelse(bh);
-
-	for (i = EXFAT_FIRST_CLUSTER; i < num_entries; i++) {
-		ep = exfat_get_dentry(sb, p_dir, entry + i, &bh);
-		if (!ep)
-			return -EIO;
 
+	for (i = ES_IDX_FIRST_FILENAME; i < num_entries; i++) {
+		ep = exfat_get_dentry_cached(es, i);
 		exfat_init_name_entry(ep, uniname);
-		exfat_update_bh(bh, sync);
-		brelse(bh);
 		uniname += EXFAT_FILE_NAME_LEN;
 	}
 
-	exfat_update_dir_chksum(inode, p_dir, entry);
-	return 0;
+	exfat_update_dir_chksum_with_entry_set(es);
 }
 
 void exfat_remove_entries(struct inode *inode, struct exfat_entry_set_cache *es,
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 46031e77f58b..0280a975586c 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -483,8 +483,8 @@ unsigned int exfat_get_entry_type(struct exfat_dentry *p_entry);
 void exfat_init_dir_entry(struct exfat_entry_set_cache *es,
 		unsigned int type, unsigned int start_clu,
 		unsigned long long size, struct timespec64 *ts);
-int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
-		int entry, int num_entries, struct exfat_uni_name *p_uniname);
+void exfat_init_ext_entry(struct exfat_entry_set_cache *es, int num_entries,
+		struct exfat_uni_name *p_uniname);
 void exfat_remove_entries(struct inode *inode, struct exfat_entry_set_cache *es,
 		int order);
 int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 9a0d8f2deea6..ee7d5fd0b16f 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -522,15 +522,12 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 		goto out;
 
 	exfat_init_dir_entry(&es, type, start_clu, clu_size, &ts);
+	exfat_init_ext_entry(&es, num_entries, &uniname);
 
 	ret = exfat_put_dentry_set(&es, IS_DIRSYNC(inode));
 	if (ret)
 		goto out;
 
-	ret = exfat_init_ext_entry(inode, p_dir, dentry, num_entries, &uniname);
-	if (ret)
-		goto out;
-
 	info->dir = *p_dir;
 	info->entry = dentry;
 	info->flags = ALLOC_NO_FAT_CHAIN;
@@ -1001,8 +998,7 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 	int ret, num_new_entries;
 	struct exfat_dentry *epold, *epnew;
 	struct super_block *sb = inode->i_sb;
-	struct buffer_head *new_bh;
-	struct exfat_entry_set_cache old_es;
+	struct exfat_entry_set_cache old_es, new_es;
 	int sync = IS_DIRSYNC(inode);
 
 	num_new_entries = exfat_calc_num_entries(p_uniname);
@@ -1027,33 +1023,25 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 			goto put_old_es;
 		}
 
-		epnew = exfat_get_dentry(sb, p_dir, newentry, &new_bh);
-		if (!epnew) {
-			ret = -EIO;
+		ret = exfat_get_empty_dentry_set(&new_es, sb, p_dir, newentry,
+				num_new_entries);
+		if (ret)
 			goto put_old_es;
-		}
 
+		epnew = exfat_get_dentry_cached(&new_es, ES_IDX_FILE);
 		*epnew = *epold;
 		if (exfat_get_entry_type(epnew) == TYPE_FILE) {
 			epnew->dentry.file.attr |= cpu_to_le16(EXFAT_ATTR_ARCHIVE);
 			ei->attr |= EXFAT_ATTR_ARCHIVE;
 		}
-		exfat_update_bh(new_bh, sync);
-		brelse(new_bh);
 
 		epold = exfat_get_dentry_cached(&old_es, ES_IDX_STREAM);
-		epnew = exfat_get_dentry(sb, p_dir, newentry + 1, &new_bh);
-		if (!epnew) {
-			ret = -EIO;
-			goto put_old_es;
-		}
-
+		epnew = exfat_get_dentry_cached(&new_es, ES_IDX_STREAM);
 		*epnew = *epold;
-		exfat_update_bh(new_bh, sync);
-		brelse(new_bh);
 
-		ret = exfat_init_ext_entry(inode, p_dir, newentry,
-			num_new_entries, p_uniname);
+		exfat_init_ext_entry(&new_es, num_new_entries, p_uniname);
+
+		ret = exfat_put_dentry_set(&new_es, sync);
 		if (ret)
 			goto put_old_es;
 
@@ -1067,11 +1055,7 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 		}
 
 		exfat_remove_entries(inode, &old_es, ES_IDX_FIRST_FILENAME + 1);
-
-		ret = exfat_init_ext_entry(inode, p_dir, oldentry,
-			num_new_entries, p_uniname);
-		if (ret)
-			goto put_old_es;
+		exfat_init_ext_entry(&old_es, num_new_entries, p_uniname);
 	}
 	return exfat_put_dentry_set(&old_es, sync);
 
@@ -1087,8 +1071,7 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 	int ret, newentry, num_new_entries;
 	struct exfat_dentry *epmov, *epnew;
 	struct super_block *sb = inode->i_sb;
-	struct buffer_head *new_bh;
-	struct exfat_entry_set_cache mov_es;
+	struct exfat_entry_set_cache mov_es, new_es;
 
 	num_new_entries = exfat_calc_num_entries(p_uniname);
 	if (num_new_entries < 0)
@@ -1103,43 +1086,35 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 	if (ret)
 		return -EIO;
 
-	epmov = exfat_get_dentry_cached(&mov_es, ES_IDX_FILE);
-	epnew = exfat_get_dentry(sb, p_newdir, newentry, &new_bh);
-	if (!epnew) {
-		ret = -EIO;
+	ret = exfat_get_empty_dentry_set(&new_es, sb, p_newdir, newentry,
+			num_new_entries);
+	if (ret)
 		goto put_mov_es;
-	}
 
+	epmov = exfat_get_dentry_cached(&mov_es, ES_IDX_FILE);
+	epnew = exfat_get_dentry_cached(&new_es, ES_IDX_FILE);
 	*epnew = *epmov;
 	if (exfat_get_entry_type(epnew) == TYPE_FILE) {
 		epnew->dentry.file.attr |= cpu_to_le16(EXFAT_ATTR_ARCHIVE);
 		ei->attr |= EXFAT_ATTR_ARCHIVE;
 	}
-	exfat_update_bh(new_bh, IS_DIRSYNC(inode));
-	brelse(new_bh);
 
 	epmov = exfat_get_dentry_cached(&mov_es, ES_IDX_STREAM);
-	epnew = exfat_get_dentry(sb, p_newdir, newentry + 1, &new_bh);
-	if (!epnew) {
-		ret = -EIO;
-		goto put_mov_es;
-	}
-
+	epnew = exfat_get_dentry_cached(&new_es, ES_IDX_STREAM);
 	*epnew = *epmov;
-	exfat_update_bh(new_bh, IS_DIRSYNC(inode));
-	brelse(new_bh);
-
-	ret = exfat_init_ext_entry(inode, p_newdir, newentry, num_new_entries,
-		p_uniname);
-	if (ret)
-		return ret;
 
+	exfat_init_ext_entry(&new_es, num_new_entries, p_uniname);
 	exfat_remove_entries(inode, &mov_es, ES_IDX_FILE);
 
 	exfat_chain_set(&ei->dir, p_newdir->dir, p_newdir->size,
 		p_newdir->flags);
 
 	ei->entry = newentry;
+
+	ret = exfat_put_dentry_set(&new_es, IS_DIRSYNC(inode));
+	if (ret)
+		goto put_mov_es;
+
 	return exfat_put_dentry_set(&mov_es, IS_DIRSYNC(inode));
 
 put_mov_es:
-- 
2.25.1


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

* [PATCH v1 08/11] exfat: remove __exfat_find_empty_entry()
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
                   ` (6 preceding siblings ...)
  2023-12-08 11:23 ` [PATCH v1 07/11] exfat: covert exfat_init_ext_entry() to use dentry cache yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-22  5:07   ` Namjae Jeon
  2023-12-08 11:23 ` [PATCH v1 09/11] exfat: remove unused functions yuezhang.mo
  2023-12-08 11:23 ` [PATCH v1 10/11] exfat: do not sync parent dir if just update timestamp yuezhang.mo
  9 siblings, 1 reply; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

This commit removes exfat_find_empty_entry() and renames
__exfat_find_empty_entry() to exfat_find_empty_entry().

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/namei.c | 49 ++++++++++++------------------------------------
 1 file changed, 12 insertions(+), 37 deletions(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index ee7d5fd0b16f..79e3fc9d6e19 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -291,7 +291,7 @@ static int exfat_check_max_dentries(struct inode *inode)
 /* find empty directory entry.
  * if there isn't any empty slot, expand cluster chain.
  */
-static int __exfat_find_empty_entry(struct inode *inode,
+static int exfat_find_empty_entry(struct inode *inode,
 		struct exfat_chain *p_dir, int num_entries,
 		struct exfat_entry_set_cache *es)
 {
@@ -382,21 +382,6 @@ static int __exfat_find_empty_entry(struct inode *inode,
 	return dentry;
 }
 
-static int exfat_find_empty_entry(struct inode *inode,
-		struct exfat_chain *p_dir, int num_entries)
-{
-	int entry;
-	struct exfat_entry_set_cache es;
-
-	entry = __exfat_find_empty_entry(inode, p_dir, num_entries, &es);
-	if (entry < 0)
-		return entry;
-
-	exfat_put_dentry_set(&es, false);
-
-	return entry;
-}
-
 /*
  * Name Resolution Functions :
  * Zero if it was successful; otherwise nonzero.
@@ -498,7 +483,7 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 	}
 
 	/* exfat_find_empty_entry must be called before alloc_cluster() */
-	dentry = exfat_find_empty_entry(inode, p_dir, num_entries);
+	dentry = exfat_find_empty_entry(inode, p_dir, num_entries, &es);
 	if (dentry < 0) {
 		ret = dentry; /* -EIO or -ENOSPC */
 		goto out;
@@ -506,8 +491,10 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 
 	if (type == TYPE_DIR && !sbi->options.zero_size_dir) {
 		ret = exfat_alloc_new_dir(inode, &clu);
-		if (ret)
+		if (ret) {
+			exfat_put_dentry_set(&es, false);
 			goto out;
+		}
 		start_clu = clu.dir;
 		clu_size = sbi->cluster_size;
 	}
@@ -516,11 +503,6 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 	/* fill the dos name directory entry information of the created file.
 	 * the first cluster is not determined yet. (0)
 	 */
-
-	ret = exfat_get_empty_dentry_set(&es, sb, p_dir, dentry, num_entries);
-	if (ret)
-		goto out;
-
 	exfat_init_dir_entry(&es, type, start_clu, clu_size, &ts);
 	exfat_init_ext_entry(&es, num_entries, &uniname);
 
@@ -1016,18 +998,13 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 	if (old_es.num_entries < num_new_entries) {
 		int newentry;
 
-		newentry =
-			exfat_find_empty_entry(inode, p_dir, num_new_entries);
+		newentry = exfat_find_empty_entry(inode, p_dir, num_new_entries,
+				&new_es);
 		if (newentry < 0) {
 			ret = newentry; /* -EIO or -ENOSPC */
 			goto put_old_es;
 		}
 
-		ret = exfat_get_empty_dentry_set(&new_es, sb, p_dir, newentry,
-				num_new_entries);
-		if (ret)
-			goto put_old_es;
-
 		epnew = exfat_get_dentry_cached(&new_es, ES_IDX_FILE);
 		*epnew = *epold;
 		if (exfat_get_entry_type(epnew) == TYPE_FILE) {
@@ -1077,19 +1054,17 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 	if (num_new_entries < 0)
 		return num_new_entries;
 
-	newentry = exfat_find_empty_entry(inode, p_newdir, num_new_entries);
-	if (newentry < 0)
-		return newentry; /* -EIO or -ENOSPC */
-
 	ret = exfat_get_dentry_set(&mov_es, sb, p_olddir, oldentry,
 			ES_ALL_ENTRIES);
 	if (ret)
 		return -EIO;
 
-	ret = exfat_get_empty_dentry_set(&new_es, sb, p_newdir, newentry,
-			num_new_entries);
-	if (ret)
+	newentry = exfat_find_empty_entry(inode, p_newdir, num_new_entries,
+			&new_es);
+	if (newentry < 0) {
+		ret = newentry; /* -EIO or -ENOSPC */
 		goto put_mov_es;
+	}
 
 	epmov = exfat_get_dentry_cached(&mov_es, ES_IDX_FILE);
 	epnew = exfat_get_dentry_cached(&new_es, ES_IDX_FILE);
-- 
2.25.1


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

* [PATCH v1 09/11] exfat: remove unused functions
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
                   ` (7 preceding siblings ...)
  2023-12-08 11:23 ` [PATCH v1 08/11] exfat: remove __exfat_find_empty_entry() yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-08 11:23 ` [PATCH v1 10/11] exfat: do not sync parent dir if just update timestamp yuezhang.mo
  9 siblings, 0 replies; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

exfat_count_ext_entries() is no longer called, remove it.
exfat_update_dir_chksum() is no longer called, remove it and
rename exfat_update_dir_chksum_with_entry_set() to it.

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/dir.c      | 60 ++-------------------------------------------
 fs/exfat/exfat_fs.h |  6 +----
 fs/exfat/inode.c    |  2 +-
 3 files changed, 4 insertions(+), 64 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 94cedc145291..c57a727d5285 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -478,41 +478,6 @@ void exfat_init_dir_entry(struct exfat_entry_set_cache *es,
 	exfat_init_stream_entry(ep, start_clu, size);
 }
 
-int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
-		int entry)
-{
-	struct super_block *sb = inode->i_sb;
-	int ret = 0;
-	int i, num_entries;
-	u16 chksum;
-	struct exfat_dentry *ep, *fep;
-	struct buffer_head *fbh, *bh;
-
-	fep = exfat_get_dentry(sb, p_dir, entry, &fbh);
-	if (!fep)
-		return -EIO;
-
-	num_entries = fep->dentry.file.num_ext + 1;
-	chksum = exfat_calc_chksum16(fep, DENTRY_SIZE, 0, CS_DIR_ENTRY);
-
-	for (i = 1; i < num_entries; i++) {
-		ep = exfat_get_dentry(sb, p_dir, entry + i, &bh);
-		if (!ep) {
-			ret = -EIO;
-			goto release_fbh;
-		}
-		chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
-				CS_DEFAULT);
-		brelse(bh);
-	}
-
-	fep->dentry.file.checksum = cpu_to_le16(chksum);
-	exfat_update_bh(fbh, IS_DIRSYNC(inode));
-release_fbh:
-	brelse(fbh);
-	return ret;
-}
-
 static void exfat_free_benign_secondary_clusters(struct inode *inode,
 		struct exfat_dentry *ep)
 {
@@ -552,7 +517,7 @@ void exfat_init_ext_entry(struct exfat_entry_set_cache *es, int num_entries,
 		uniname += EXFAT_FILE_NAME_LEN;
 	}
 
-	exfat_update_dir_chksum_with_entry_set(es);
+	exfat_update_dir_chksum(es);
 }
 
 void exfat_remove_entries(struct inode *inode, struct exfat_entry_set_cache *es,
@@ -574,7 +539,7 @@ void exfat_remove_entries(struct inode *inode, struct exfat_entry_set_cache *es,
 		es->modified = true;
 }
 
-void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)
+void exfat_update_dir_chksum(struct exfat_entry_set_cache *es)
 {
 	int chksum_type = CS_DIR_ENTRY, i;
 	unsigned short chksum = 0;
@@ -1237,27 +1202,6 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
 	return dentry - num_ext;
 }
 
-int exfat_count_ext_entries(struct super_block *sb, struct exfat_chain *p_dir,
-		int entry, struct exfat_dentry *ep)
-{
-	int i, count = 0;
-	unsigned int type;
-	struct exfat_dentry *ext_ep;
-	struct buffer_head *bh;
-
-	for (i = 0, entry++; i < ep->dentry.file.num_ext; i++, entry++) {
-		ext_ep = exfat_get_dentry(sb, p_dir, entry, &bh);
-		if (!ext_ep)
-			return -EIO;
-
-		type = exfat_get_entry_type(ext_ep);
-		brelse(bh);
-		if (type & TYPE_CRITICAL_SEC || type & TYPE_BENIGN_SEC)
-			count++;
-	}
-	return count;
-}
-
 int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir)
 {
 	int i, count = 0;
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 0280a975586c..a2c70b3a27bd 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -431,8 +431,6 @@ int exfat_ent_get(struct super_block *sb, unsigned int loc,
 		unsigned int *content);
 int exfat_ent_set(struct super_block *sb, unsigned int loc,
 		unsigned int content);
-int exfat_count_ext_entries(struct super_block *sb, struct exfat_chain *p_dir,
-		int entry, struct exfat_dentry *p_entry);
 int exfat_chain_cont_cluster(struct super_block *sb, unsigned int chain,
 		unsigned int len);
 int exfat_zeroed_cluster(struct inode *dir, unsigned int clu);
@@ -487,9 +485,7 @@ void exfat_init_ext_entry(struct exfat_entry_set_cache *es, int num_entries,
 		struct exfat_uni_name *p_uniname);
 void exfat_remove_entries(struct inode *inode, struct exfat_entry_set_cache *es,
 		int order);
-int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
-		int entry);
-void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es);
+void exfat_update_dir_chksum(struct exfat_entry_set_cache *es);
 int exfat_calc_num_entries(struct exfat_uni_name *p_uniname);
 int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
 		struct exfat_chain *p_dir, struct exfat_uni_name *p_uniname,
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index 522edcbb2ce4..0614bccfbe76 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -94,7 +94,7 @@ int __exfat_write_inode(struct inode *inode, int sync)
 		ep2->dentry.stream.start_clu = EXFAT_FREE_CLUSTER;
 	}
 
-	exfat_update_dir_chksum_with_entry_set(&es);
+	exfat_update_dir_chksum(&es);
 	return exfat_put_dentry_set(&es, sync);
 }
 
-- 
2.25.1


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

* [PATCH v1 10/11] exfat: do not sync parent dir if just update timestamp
       [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
                   ` (8 preceding siblings ...)
  2023-12-08 11:23 ` [PATCH v1 09/11] exfat: remove unused functions yuezhang.mo
@ 2023-12-08 11:23 ` yuezhang.mo
  2023-12-22  5:08   ` Namjae Jeon
  9 siblings, 1 reply; 15+ messages in thread
From: yuezhang.mo @ 2023-12-08 11:23 UTC (permalink / raw)
  To: linkinjeon, sj1557.seo; +Cc: linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

From: Yuezhang Mo <Yuezhang.Mo@sony.com>

When sync or dir_sync is enabled, there is no need to sync the
parent directory's inode if only for updating its timestamp.

1. If an unexpected power failure occurs, the timestamp of the
   parent directory is not updated to the storage, which has no
   impact on the user.

2. The number of writes will be greatly reduced, which can not
   only improve performance, but also prolong device life.

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/namei.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 79e3fc9d6e19..b33497845a06 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -547,6 +547,7 @@ static int exfat_create(struct mnt_idmap *idmap, struct inode *dir,
 	struct exfat_dir_entry info;
 	loff_t i_pos;
 	int err;
+	loff_t size = i_size_read(dir);
 
 	mutex_lock(&EXFAT_SB(sb)->s_lock);
 	exfat_set_volume_dirty(sb);
@@ -557,7 +558,7 @@ static int exfat_create(struct mnt_idmap *idmap, struct inode *dir,
 
 	inode_inc_iversion(dir);
 	inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
-	if (IS_DIRSYNC(dir))
+	if (IS_DIRSYNC(dir) && size != i_size_read(dir))
 		exfat_sync_inode(dir);
 	else
 		mark_inode_dirty(dir);
@@ -801,10 +802,7 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 	inode_inc_iversion(dir);
 	simple_inode_init_ts(dir);
 	exfat_truncate_inode_atime(dir);
-	if (IS_DIRSYNC(dir))
-		exfat_sync_inode(dir);
-	else
-		mark_inode_dirty(dir);
+	mark_inode_dirty(dir);
 
 	clear_nlink(inode);
 	simple_inode_init_ts(inode);
@@ -825,6 +823,7 @@ static int exfat_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	struct exfat_chain cdir;
 	loff_t i_pos;
 	int err;
+	loff_t size = i_size_read(dir);
 
 	mutex_lock(&EXFAT_SB(sb)->s_lock);
 	exfat_set_volume_dirty(sb);
@@ -835,7 +834,7 @@ static int exfat_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 
 	inode_inc_iversion(dir);
 	inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
-	if (IS_DIRSYNC(dir))
+	if (IS_DIRSYNC(dir) && size != i_size_read(dir))
 		exfat_sync_inode(dir);
 	else
 		mark_inode_dirty(dir);
@@ -1239,6 +1238,7 @@ static int exfat_rename(struct mnt_idmap *idmap,
 	struct super_block *sb = old_dir->i_sb;
 	loff_t i_pos;
 	int err;
+	loff_t size = i_size_read(new_dir);
 
 	/*
 	 * The VFS already checks for existence, so for local filesystems
@@ -1260,7 +1260,7 @@ static int exfat_rename(struct mnt_idmap *idmap,
 	simple_rename_timestamp(old_dir, old_dentry, new_dir, new_dentry);
 	EXFAT_I(new_dir)->i_crtime = current_time(new_dir);
 	exfat_truncate_inode_atime(new_dir);
-	if (IS_DIRSYNC(new_dir))
+	if (IS_DIRSYNC(new_dir) && size != i_size_read(new_dir))
 		exfat_sync_inode(new_dir);
 	else
 		mark_inode_dirty(new_dir);
@@ -1281,10 +1281,7 @@ static int exfat_rename(struct mnt_idmap *idmap,
 	}
 
 	inode_inc_iversion(old_dir);
-	if (IS_DIRSYNC(old_dir))
-		exfat_sync_inode(old_dir);
-	else
-		mark_inode_dirty(old_dir);
+	mark_inode_dirty(old_dir);
 
 	if (new_inode) {
 		exfat_unhash_inode(new_inode);
-- 
2.25.1


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

* Re: [PATCH v1 01/11] exfat: add __exfat_get_dentry_set() helper
  2023-12-08 11:23 ` [PATCH v1 01/11] exfat: add __exfat_get_dentry_set() helper yuezhang.mo
@ 2023-12-22  5:05   ` Namjae Jeon
  0 siblings, 0 replies; 15+ messages in thread
From: Namjae Jeon @ 2023-12-22  5:05 UTC (permalink / raw)
  To: yuezhang.mo
  Cc: sj1557.seo, linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

> - * Returns a set of dentries for a file or dir.
> + * Returns a set of dentries.
>   *
>   * Note It provides a direct pointer to bh->data via
> exfat_get_dentry_cached().
>   * User should call exfat_get_dentry_set() after setting 'modified' to
> apply
> @@ -842,22 +836,24 @@ struct exfat_dentry *exfat_get_dentry_cached(
>   *
>   * in:
>   *   sb+p_dir+entry: indicates a file/dir
> - *   type:  specifies how many dentries should be included.
> + *   num_entries: specifies how many dentries should be included.
> + *                It will be set to es->num_entries if it is not 0.
> + *                If num_entries is 0, es->num_entries will be obtained
> + *                from the first dentry.
> + * out:
> + *   es: pointer of entry set on success.
>   * return:
> - *   pointer of entry set on success,
> - *   NULL on failure.
> + *   0 on success
> + *   < 0 on failure
         -error code on failure.
>   */
> -int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
> +static int __exfat_get_dentry_set(struct exfat_entry_set_cache *es,
>  		struct super_block *sb, struct exfat_chain *p_dir, int entry,
> -		unsigned int type)
> +		unsigned int num_entries)
>  {
>  	int ret, i, num_bh;
>  	unsigned int off;
>  	sector_t sec;
>  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
> -	struct exfat_dentry *ep;
> -	int num_entries;
> -	enum exfat_validate_dentry_mode mode = ES_MODE_STARTED;
>  	struct buffer_head *bh;
>
>  	if (p_dir->dir == DIR_DELETED) {
> @@ -880,12 +876,16 @@ int exfat_get_dentry_set(struct exfat_entry_set_cache
> *es,
>  		return -EIO;
>  	es->bh[es->num_bh++] = bh;
>
> -	ep = exfat_get_dentry_cached(es, ES_IDX_FILE);
> -	if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
> -		goto put_es;
> +	if (num_entries == ES_ALL_ENTRIES) {
> +		struct exfat_dentry *ep;
> +
> +		ep = exfat_get_dentry_cached(es, ES_IDX_FILE);
> +		if (ep->type == EXFAT_FILE)
> +			num_entries = ep->dentry.file.num_ext + 1;
> +		else
> +			goto put_es;
I prefer to avoid else{}.
               if (ep->type != EXFAT_FILE)
                        goto put_es;
> +	}
>
> -	num_entries = type == ES_ALL_ENTRIES ?
> -		ep->dentry.file.num_ext + 1 : type;
>  	es->num_entries = num_entries;
>
>  	num_bh = EXFAT_B_TO_BLK_ROUND_UP(off + num_entries * DENTRY_SIZE, sb);
> @@ -918,8 +918,27 @@ int exfat_get_dentry_set(struct exfat_entry_set_cache
> *es,
>  		es->bh[es->num_bh++] = bh;
>  	}
>
> +	return 0;
> +
> +put_es:
> +	exfat_put_dentry_set(es, false);
> +	return -EIO;
> +}
> +
> +int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
> +		struct super_block *sb, struct exfat_chain *p_dir,
> +		int entry, unsigned int type)
> +{
> +	int ret, i;
> +	struct exfat_dentry *ep;
> +	enum exfat_validate_dentry_mode mode = ES_MODE_GET_FILE_ENTRY;
> +
> +	ret = __exfat_get_dentry_set(es, sb, p_dir, entry, type);
You need to change type to num_entries ?

> +	if (ret < 0)
> +		return ret;
> +
>  	/* validate cached dentries */
> -	for (i = ES_IDX_STREAM; i < num_entries; i++) {
> +	for (i = ES_IDX_STREAM; i < es->num_entries; i++) {
>  		ep = exfat_get_dentry_cached(es, i);
>  		if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
>  			goto put_es;
> --
> 2.25.1
>
>

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

* Re: [PATCH v1 08/11] exfat: remove __exfat_find_empty_entry()
  2023-12-08 11:23 ` [PATCH v1 08/11] exfat: remove __exfat_find_empty_entry() yuezhang.mo
@ 2023-12-22  5:07   ` Namjae Jeon
  0 siblings, 0 replies; 15+ messages in thread
From: Namjae Jeon @ 2023-12-22  5:07 UTC (permalink / raw)
  To: yuezhang.mo
  Cc: sj1557.seo, linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

2023-12-08 20:23 GMT+09:00, yuezhang.mo@foxmail.com <yuezhang.mo@foxmail.com>:
> From: Yuezhang Mo <Yuezhang.Mo@sony.com>
>
> This commit removes exfat_find_empty_entry() and renames
> __exfat_find_empty_entry() to exfat_find_empty_entry().
>
> 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>
If you update 0003 patch, this patch is not needed. I don't understand
why you delete function again after adding it in previous patch.

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

* Re: [PATCH v1 05/11] exfat: covert exfat_remove_entries() to use dentry cache
  2023-12-08 11:23 ` [PATCH v1 05/11] exfat: covert exfat_remove_entries() " yuezhang.mo
@ 2023-12-22  5:07   ` Namjae Jeon
  0 siblings, 0 replies; 15+ messages in thread
From: Namjae Jeon @ 2023-12-22  5:07 UTC (permalink / raw)
  To: yuezhang.mo
  Cc: sj1557.seo, linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

2023-12-08 20:23 GMT+09:00, yuezhang.mo@foxmail.com <yuezhang.mo@foxmail.com>:
> From: Yuezhang Mo <Yuezhang.Mo@sony.com>
>
> Before this conversion, in exfat_remove_entries(), to mark the
> dentries in a dentry set as deleted, the sync times is equals
> the dentry numbers if 'dirsync' or 'sync' is enabled.
> That affects not only performance but also device life.
>
> After this conversion, only needs to be synchronized once if
> 'dirsync' or 'sync' is enabled.
s/covert/convert in patch subject ?

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

* Re: [PATCH v1 10/11] exfat: do not sync parent dir if just update timestamp
  2023-12-08 11:23 ` [PATCH v1 10/11] exfat: do not sync parent dir if just update timestamp yuezhang.mo
@ 2023-12-22  5:08   ` Namjae Jeon
  2023-12-24 14:58     ` Yuezhang.Mo
  0 siblings, 1 reply; 15+ messages in thread
From: Namjae Jeon @ 2023-12-22  5:08 UTC (permalink / raw)
  To: yuezhang.mo
  Cc: sj1557.seo, linux-fsdevel, Andy.Wu, wataru.aoyama, Yuezhang Mo

2023-12-08 20:23 GMT+09:00, yuezhang.mo@foxmail.com <yuezhang.mo@foxmail.com>:
> From: Yuezhang Mo <Yuezhang.Mo@sony.com>
>
> When sync or dir_sync is enabled, there is no need to sync the
> parent directory's inode if only for updating its timestamp.
>
> 1. If an unexpected power failure occurs, the timestamp of the
>    parent directory is not updated to the storage, which has no
>    impact on the user.
Well, Why do you think timestamp sync of parent dir is not important ?
>
> 2. The number of writes will be greatly reduced, which can not
>    only improve performance, but also prolong device life.
How much does this have on your measurement results?

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

* RE: [PATCH v1 10/11] exfat: do not sync parent dir if just update timestamp
  2023-12-22  5:08   ` Namjae Jeon
@ 2023-12-24 14:58     ` Yuezhang.Mo
  0 siblings, 0 replies; 15+ messages in thread
From: Yuezhang.Mo @ 2023-12-24 14:58 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: sj1557.seo@samsung.com, linux-fsdevel@vger.kernel.org,
	Andy.Wu@sony.com, Wataru.Aoyama@sony.com

> From: Namjae Jeon <linkinjeon@kernel.org>
> Sent: Friday, December 22, 2023 1:09 PM
> > From: Yuezhang Mo <mailto:Yuezhang.Mo@sony.com>
> >
> > When sync or dir_sync is enabled, there is no need to sync the parent
> > directory's inode if only for updating its timestamp.
> >
> > 1. If an unexpected power failure occurs, the timestamp of the
> >    parent directory is not updated to the storage, which has no
> >    impact on the user.
> Well, Why do you think timestamp sync of parent dir is not important ?

If the size of the parent directory does not change (only the timestamp of
the parent directory needs to be synchronized), synchronizing the dentry
set of the parent directory is not as important as synchronizing the dentry
set of the target file.

In the case of a large number of file operations, skipping the synchronization
of the parent directory timestamp can allow the file operations to be
completed faster and shorten the file operation completion time. The risk of
user data loss is greater if there is a possibility of unexpected power outage
and the process is not completed for a long time.

By skipping the synchronization of the parent directory timestamp, even if
the parent directory timestamp is not updated due to an unexpected power
outage, the file operation will still be successful.

> >
> > 2. The number of writes will be greatly reduced, which can not
> >    only improve performance, but also prolong device life.
> How much does this have on your measurement results?

I did not measure the improvements brought by this patch alone,
the improvements brought by this patch set are listed in [0/11].

If without this patch, 2 dentry sets(the target file dentry set and the parent
dir dentry set) will be sync in a file operation.

If with this patch, 1 dentry set(the target file dentry set) will be sync in a
file operation, the parent dir dentry set will not be sync in most case.

So this patch reduces metadata synchronization by nearly half.

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

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

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20231208112318.1135649-1-yuezhang.mo@foxmail.com>
2023-12-08 11:23 ` [PATCH v1 01/11] exfat: add __exfat_get_dentry_set() helper yuezhang.mo
2023-12-22  5:05   ` Namjae Jeon
2023-12-08 11:23 ` [PATCH v1 02/11] exfat: add exfat_get_empty_dentry_set() helper yuezhang.mo
2023-12-08 11:23 ` [PATCH v1 03/11] exfat: covert exfat_find_empty_entry() to use dentry cache yuezhang.mo
2023-12-08 11:23 ` [PATCH v1 04/11] exfat: covert exfat_add_entry() " yuezhang.mo
2023-12-08 11:23 ` [PATCH v1 05/11] exfat: covert exfat_remove_entries() " yuezhang.mo
2023-12-22  5:07   ` Namjae Jeon
2023-12-08 11:23 ` [PATCH v1 06/11] exfat: move free cluster out of exfat_init_ext_entry() yuezhang.mo
2023-12-08 11:23 ` [PATCH v1 07/11] exfat: covert exfat_init_ext_entry() to use dentry cache yuezhang.mo
2023-12-08 11:23 ` [PATCH v1 08/11] exfat: remove __exfat_find_empty_entry() yuezhang.mo
2023-12-22  5:07   ` Namjae Jeon
2023-12-08 11:23 ` [PATCH v1 09/11] exfat: remove unused functions yuezhang.mo
2023-12-08 11:23 ` [PATCH v1 10/11] exfat: do not sync parent dir if just update timestamp yuezhang.mo
2023-12-22  5:08   ` Namjae Jeon
2023-12-24 14:58     ` 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 NNTP newsgroup(s).