[PATCH v2] erofs-utils: update i_nlink stat for directories

* [PATCH v2] erofs-utils: update i_nlink stat for directories
       [not found] <20201205091637.8944-1-hsiangkao.ref@aol.com>
@ 2020-12-05  9:16 ` Gao Xiang via Linux-erofs
  2020-12-05  9:22   ` Li GuiFu via Linux-erofs
  0 siblings, 1 reply; 3+ messages in thread
From: Gao Xiang via Linux-erofs @ 2020-12-05  9:16 UTC (permalink / raw)
  To: linux-erofs, Li Guifu

From: Gao Xiang <hsiangkao@aol.com>

Previously, nlink of directories was treated as 1 for simplicity.

Since st_nlink for dirs is actually not well defined, nlink=1 seems
to pacify `find' (even without -noleaf option) and other utilities.
AFAICT, isofs, romfs and cramfs always set it to 1, Overlayfs sets
it to 1 conditionally, btrfs[1], ceph[2] and FUSE client historically
set it to 1.

The convention under unix is that it's # of subdirs including "."
and "..". This patch tries to follow such convention if possible to
optimize `find' performance since it's not quite hard for local fs.

[1] https://lore.kernel.org/r/20100124003336.GP23006@think
[2] https://lore.kernel.org/r/20180521092729.17470-1-lhenriques@suse.com
Signed-off-by: Gao Xiang <hsiangkao@aol.com>
---
v1: https://lore.kernel.org/r/20201205055732.14276-1-hsiangkao@aol.com
changes since v1:
 - update a DBG_BUGON statement suggestted by Guifu.

 lib/inode.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/lib/inode.c b/lib/inode.c
index 618eb284550f..3d634fc92852 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -25,7 +25,7 @@
 struct erofs_sb_info sbi;
 
 #define S_SHIFT                 12
-static unsigned char erofs_type_by_mode[S_IFMT >> S_SHIFT] = {
+static unsigned char erofs_ftype_by_mode[S_IFMT >> S_SHIFT] = {
 	[S_IFREG >> S_SHIFT]  = EROFS_FT_REG_FILE,
 	[S_IFDIR >> S_SHIFT]  = EROFS_FT_DIR,
 	[S_IFCHR >> S_SHIFT]  = EROFS_FT_CHRDEV,
@@ -35,6 +35,11 @@ static unsigned char erofs_type_by_mode[S_IFMT >> S_SHIFT] = {
 	[S_IFLNK >> S_SHIFT]  = EROFS_FT_SYMLINK,
 };
 
+static unsigned char erofs_mode_to_ftype(umode_t mode)
+{
+	return erofs_ftype_by_mode[(mode & S_IFMT) >> S_SHIFT];
+}
+
 #define NR_INODE_HASHTABLE	16384
 
 struct list_head inode_hashtable[NR_INODE_HASHTABLE];
@@ -156,7 +161,7 @@ static int __allocate_inode_bh_data(struct erofs_inode *inode,
 int erofs_prepare_dir_file(struct erofs_inode *dir)
 {
 	struct erofs_dentry *d;
-	unsigned int d_size;
+	unsigned int d_size, i_nlink;
 	int ret;
 
 	/* dot is pointed to the current dir inode */
@@ -169,16 +174,28 @@ int erofs_prepare_dir_file(struct erofs_inode *dir)
 	d->inode = erofs_igrab(dir->i_parent);
 	d->type = EROFS_FT_DIR;
 
-	/* let's calculate dir size */
+	/* let's calculate dir size and update i_nlink */
 	d_size = 0;
+	i_nlink = 0;
 	list_for_each_entry(d, &dir->i_subdirs, d_child) {
 		int len = strlen(d->name) + sizeof(struct erofs_dirent);
 
 		if (d_size % EROFS_BLKSIZ + len > EROFS_BLKSIZ)
 			d_size = round_up(d_size, EROFS_BLKSIZ);
 		d_size += len;
+
+		i_nlink += (d->type == EROFS_FT_DIR);
 	}
 	dir->i_size = d_size;
+	/*
+	 * if there're too many subdirs as compact form, set nlink=1
+	 * rather than upgrade to use extented form instead.
+	 */
+	if (i_nlink > USHRT_MAX &&
+	    dir->inode_isize == sizeof(struct erofs_inode_compact))
+		dir->i_nlink = 1;
+	else
+		dir->i_nlink = i_nlink;
 
 	/* no compression for all dirs */
 	dir->datalayout = EROFS_INODE_FLAT_INLINE;
@@ -957,6 +974,10 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 			ret = PTR_ERR(d);
 			goto err_closedir;
 		}
+
+		/* to count i_nlink for directories */
+		d->type = (dp->d_type == DT_DIR ?
+			EROFS_FT_DIR : EROFS_FT_UNKNOWN);
 	}
 
 	if (errno) {
@@ -978,6 +999,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 
 	list_for_each_entry(d, &dir->i_subdirs, d_child) {
 		char buf[PATH_MAX];
+		unsigned char ftype;
 
 		if (is_dot_dotdot(d->name)) {
 			erofs_d_invalidate(d);
@@ -1000,7 +1022,10 @@ fail:
 			goto err;
 		}
 
-		d->type = erofs_type_by_mode[d->inode->i_mode >> S_SHIFT];
+		ftype = erofs_mode_to_ftype(d->inode->i_mode);
+		DBG_BUGON(ftype == EROFS_FT_DIR && d->type != ftype);
+		d->type = ftype;
+
 		erofs_d_invalidate(d);
 		erofs_info("add file %s/%s (nid %llu, type %d)",
 			   dir->i_srcpath, d->name, (unsigned long long)d->nid,
-- 
2.24.0


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