[PATCH v1 1/2] erofs-utils: add list_replace from linux kernel for dirs sorted later

[PATCH v1 1/2] erofs-utils: add list_replace from linux kernel for dirs sorted later

[Li Guifu via Linux-erofs]

A temp list head will be replaced to inode i_subdirs.

Signed-off-by: Li Guifu <bluce.lee@aliyun.com>
---
 include/erofs/list.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/erofs/list.h b/include/erofs/list.h
index 3572726..7238418 100644
--- a/include/erofs/list.h
+++ b/include/erofs/list.h
@@ -67,6 +67,22 @@ static inline int list_empty(struct list_head *head)
 	return head->next == head;
 }
 
+/**
+ * list_replace - replace old entry by new one
+ * @old : the element to be replaced
+ * @new : the new element to insert
+ *
+ * If @old was empty, it will be overwritten.
+ */
+static inline void list_replace(struct list_head *old,
+				struct list_head *new)
+{
+	new->next = old->next;
+	new->next->prev = new;
+	new->prev = old->prev;
+	new->prev->next = new;
+}
+
 #define list_entry(ptr, type, member) container_of(ptr, type, member)
 
 #define list_first_entry(ptr, type, member)                                    \
-- 
2.17.1

[PATCH v1 2/2] erofs-utils: introduce erofs_subdirs to one dir for sort

[Li Guifu via Linux-erofs]

The structure erofs_subdirs has a dir number and a list_head,
the list_head is the same with i_subdirs in the inode.
Using erofs_subdirs as a temp place for dentrys under the dir,
and then sort it before replace to i_subdirs

Signed-off-by: Li Guifu <bluce.lee@aliyun.com>
---
 include/erofs/internal.h |  5 +++
 lib/inode.c              | 95 +++++++++++++++++++++++++---------------
 2 files changed, 64 insertions(+), 36 deletions(-)

diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index 1339341..7cd42ca 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -172,6 +172,11 @@ struct erofs_inode {
 #endif
 };
 
+struct erofs_subdirs {
+	struct list_head i_subdirs;
+	unsigned int nr_subdirs;
+};
+
 static inline bool is_inode_layout_compression(struct erofs_inode *inode)
 {
 	return erofs_inode_is_data_compressed(inode->datalayout);
diff --git a/lib/inode.c b/lib/inode.c
index 787e5b4..3e138a6 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -96,7 +96,7 @@ unsigned int erofs_iput(struct erofs_inode *inode)
 	return 0;
 }
 
-struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent,
+struct erofs_dentry *erofs_d_alloc(struct erofs_subdirs *subdirs,
 				   const char *name)
 {
 	struct erofs_dentry *d = malloc(sizeof(*d));
@@ -107,7 +107,8 @@ struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent,
 	strncpy(d->name, name, EROFS_NAME_LEN - 1);
 	d->name[EROFS_NAME_LEN - 1] = '\0';
 
-	list_add_tail(&d->d_child, &parent->i_subdirs);
+	list_add_tail(&d->d_child, &subdirs->i_subdirs);
+	subdirs->nr_subdirs++;
 	return d;
 }
 
@@ -150,38 +151,12 @@ static int comp_subdir(const void *a, const void *b)
 	return strcmp(da->name, db->name);
 }
 
-int erofs_prepare_dir_file(struct erofs_inode *dir, unsigned int nr_subdirs)
+int erofs_prepare_dir_file(struct erofs_inode *dir)
 {
-	struct erofs_dentry *d, *n, **sorted_d;
-	unsigned int d_size, i_nlink, i;
+	struct erofs_dentry *d;
+	unsigned int d_size, i_nlink;
 	int ret;
 
-	/* dot is pointed to the current dir inode */
-	d = erofs_d_alloc(dir, ".");
-	d->inode = erofs_igrab(dir);
-	d->type = EROFS_FT_DIR;
-
-	/* dotdot is pointed to the parent dir */
-	d = erofs_d_alloc(dir, "..");
-	d->inode = erofs_igrab(dir->i_parent);
-	d->type = EROFS_FT_DIR;
-
-	/* sort subdirs */
-	nr_subdirs += 2;
-	sorted_d = malloc(nr_subdirs * sizeof(d));
-	if (!sorted_d)
-		return -ENOMEM;
-	i = 0;
-	list_for_each_entry_safe(d, n, &dir->i_subdirs, d_child) {
-		list_del(&d->d_child);
-		sorted_d[i++] = d;
-	}
-	DBG_BUGON(i != nr_subdirs);
-	qsort(sorted_d, nr_subdirs, sizeof(d), comp_subdir);
-	for (i = 0; i < nr_subdirs; i++)
-		list_add_tail(&sorted_d[i]->d_child, &dir->i_subdirs);
-	free(sorted_d);
-
 	/* let's calculate dir size and update i_nlink */
 	d_size = 0;
 	i_nlink = 0;
@@ -926,13 +901,58 @@ void erofs_d_invalidate(struct erofs_dentry *d)
 	erofs_iput(inode);
 }
 
+void erofs_subdirs_init(struct erofs_inode *dir, struct erofs_subdirs *subdirs)
+{
+	struct erofs_dentry *d;
+
+	subdirs->nr_subdirs = 0;
+	init_list_head(&subdirs->i_subdirs);
+
+	/* dot is pointed to the current dir inode */
+	d = erofs_d_alloc(subdirs, ".");
+	d->inode = erofs_igrab(dir);
+	d->type = EROFS_FT_DIR;
+
+	/* dotdot is pointed to the parent dir */
+	d = erofs_d_alloc(subdirs, "..");
+	d->inode = erofs_igrab(dir->i_parent);
+	d->type = EROFS_FT_DIR;
+}
+
+static int erofs_subdirs_sorted(struct erofs_subdirs *subdirs)
+{
+	struct erofs_dentry *d, *n, **sorted_d;
+	unsigned int i;
+	const unsigned int nr_subdirs = subdirs->nr_subdirs;
+
+	if (nr_subdirs == 0) return 0;
+
+	sorted_d = malloc(nr_subdirs * sizeof(d));
+	if (!sorted_d)
+		return -ENOMEM;
+	i = 0;
+	list_for_each_entry_safe(d, n, &subdirs->i_subdirs, d_child) {
+		list_del(&d->d_child);
+		sorted_d[i++] = d;
+	}
+
+	DBG_BUGON(i != nr_subdirs);
+	DBG_BUGON(!list_empty(&subdirs->i_subdirs));
+
+	qsort(sorted_d, nr_subdirs, sizeof(d), comp_subdir);
+	for (i = 0; i < nr_subdirs; i++)
+		list_add_tail(&sorted_d[i]->d_child, &subdirs->i_subdirs);
+	free(sorted_d);
+	return 0;
+}
+
 struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 {
 	int ret;
 	DIR *_dir;
 	struct dirent *dp;
 	struct erofs_dentry *d;
-	unsigned int nr_subdirs;
+	struct erofs_subdirs subdirs;
 
 	ret = erofs_prepare_xattr_ibody(dir);
 	if (ret < 0)
@@ -972,7 +992,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 		return ERR_PTR(-errno);
 	}
 
-	nr_subdirs = 0;
+	erofs_subdirs_init(dir, &subdirs);
 	while (1) {
 		/*
 		 * set errno to 0 before calling readdir() in order to
@@ -991,12 +1011,11 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 		if (erofs_is_exclude_path(dir->i_srcpath, dp->d_name))
 			continue;
 
-		d = erofs_d_alloc(dir, dp->d_name);
+		d = erofs_d_alloc(&subdirs, dp->d_name);
 		if (IS_ERR(d)) {
 			ret = PTR_ERR(d);
 			goto err_closedir;
 		}
-		nr_subdirs++;
 
 		/* to count i_nlink for directories */
 		d->type = (dp->d_type == DT_DIR ?
@@ -1009,7 +1028,11 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 	}
 	closedir(_dir);
 
-	ret = erofs_prepare_dir_file(dir, nr_subdirs);
+	ret = erofs_subdirs_sorted(&subdirs);
+	if (ret) goto err;
+
+	list_replace(&subdirs.i_subdirs, &dir->i_subdirs);
+	ret = erofs_prepare_dir_file(dir);
 	if (ret)
 		goto err;
 
-- 
2.17.1

Re: [PATCH v1 2/2] erofs-utils: introduce erofs_subdirs to one dir for sort

[Gao Xiang]

Hi Guifu,

On Wed, May 05, 2021 at 10:26:15PM +0800, Li Guifu via Linux-erofs wrote:
> The structure erofs_subdirs has a dir number and a list_head,
> the list_head is the same with i_subdirs in the inode.
> Using erofs_subdirs as a temp place for dentrys under the dir,
> and then sort it before replace to i_subdirs
> 
> Signed-off-by: Li Guifu <bluce.lee@aliyun.com>
> ---
>  include/erofs/internal.h |  5 +++
>  lib/inode.c              | 95 +++++++++++++++++++++++++---------------
>  2 files changed, 64 insertions(+), 36 deletions(-)
> 
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index 1339341..7cd42ca 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -172,6 +172,11 @@ struct erofs_inode {
>  #endif
>  };
>  
> +struct erofs_subdirs {
> +	struct list_head i_subdirs;
> +	unsigned int nr_subdirs;
> +};
> +
>  static inline bool is_inode_layout_compression(struct erofs_inode *inode)
>  {
>  	return erofs_inode_is_data_compressed(inode->datalayout);
> diff --git a/lib/inode.c b/lib/inode.c
> index 787e5b4..3e138a6 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -96,7 +96,7 @@ unsigned int erofs_iput(struct erofs_inode *inode)
>  	return 0;
>  }
>  
> -struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent,
> +struct erofs_dentry *erofs_d_alloc(struct erofs_subdirs *subdirs,
>  				   const char *name)
>  {
>  	struct erofs_dentry *d = malloc(sizeof(*d));
> @@ -107,7 +107,8 @@ struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent,
>  	strncpy(d->name, name, EROFS_NAME_LEN - 1);
>  	d->name[EROFS_NAME_LEN - 1] = '\0';
>  
> -	list_add_tail(&d->d_child, &parent->i_subdirs);
> +	list_add_tail(&d->d_child, &subdirs->i_subdirs);
> +	subdirs->nr_subdirs++;
>  	return d;
>  }
>  
> @@ -150,38 +151,12 @@ static int comp_subdir(const void *a, const void *b)
>  	return strcmp(da->name, db->name);
>  }
>  
> -int erofs_prepare_dir_file(struct erofs_inode *dir, unsigned int nr_subdirs)
> +int erofs_prepare_dir_file(struct erofs_inode *dir)

Err... nope, that is not what I suggested, I was suggesting

int erofs_prepare_dir_file(struct erofs_inode *dir,
			   struct erofs_subdirs *subdirs)

>  {
> -	struct erofs_dentry *d, *n, **sorted_d;
> -	unsigned int d_size, i_nlink, i;
> +	struct erofs_dentry *d;
> +	unsigned int d_size, i_nlink;
>  	int ret;
>  
> -	/* dot is pointed to the current dir inode */
> -	d = erofs_d_alloc(dir, ".");
> -	d->inode = erofs_igrab(dir);
> -	d->type = EROFS_FT_DIR;
> -
> -	/* dotdot is pointed to the parent dir */
> -	d = erofs_d_alloc(dir, "..");
> -	d->inode = erofs_igrab(dir->i_parent);
> -	d->type = EROFS_FT_DIR;

Leave these two here, since it's a part of dir preparation.

> -
> -	/* sort subdirs */
> -	nr_subdirs += 2;
> -	sorted_d = malloc(nr_subdirs * sizeof(d));
> -	if (!sorted_d)
> -		return -ENOMEM;
> -	i = 0;
> -	list_for_each_entry_safe(d, n, &dir->i_subdirs, d_child) {
> -		list_del(&d->d_child);
> -		sorted_d[i++] = d;
> -	}
> -	DBG_BUGON(i != nr_subdirs);
> -	qsort(sorted_d, nr_subdirs, sizeof(d), comp_subdir);
> -	for (i = 0; i < nr_subdirs; i++)
> -		list_add_tail(&sorted_d[i]->d_child, &dir->i_subdirs);
> -	free(sorted_d);
> -
>  	/* let's calculate dir size and update i_nlink */
>  	d_size = 0;
>  	i_nlink = 0;
> @@ -926,13 +901,58 @@ void erofs_d_invalidate(struct erofs_dentry *d)
>  	erofs_iput(inode);
>  }
>  
> +void erofs_subdirs_init(struct erofs_inode *dir, struct erofs_subdirs *subdirs)
> +{
> +	struct erofs_dentry *d;
> +
> +	subdirs->nr_subdirs = 0;
> +	init_list_head(&subdirs->i_subdirs);
> +
> +	/* dot is pointed to the current dir inode */
> +	d = erofs_d_alloc(subdirs, ".");
> +	d->inode = erofs_igrab(dir);
> +	d->type = EROFS_FT_DIR;
> +
> +	/* dotdot is pointed to the parent dir */
> +	d = erofs_d_alloc(subdirs, "..");
> +	d->inode = erofs_igrab(dir->i_parent);
> +	d->type = EROFS_FT_DIR;
> +}

No need of this, just
struct erofs_subdirs subdirs = {0}; in erofs_mkfs_build_tree().

> +
> +static int erofs_subdirs_sorted(struct erofs_subdirs *subdirs)
> +{
> +	struct erofs_dentry *d, *n, **sorted_d;
> +	unsigned int i;
> +	const unsigned int nr_subdirs = subdirs->nr_subdirs;
> +
> +	if (nr_subdirs == 0) return 0;
> +
> +	sorted_d = malloc(nr_subdirs * sizeof(d));
> +	if (!sorted_d)
> +		return -ENOMEM;
> +	i = 0;
> +	list_for_each_entry_safe(d, n, &subdirs->i_subdirs, d_child) {
> +		list_del(&d->d_child);
> +		sorted_d[i++] = d;
> +	}
> +
> +	DBG_BUGON(i != nr_subdirs);
> +	DBG_BUGON(!list_empty(&subdirs->i_subdirs));
> +
> +	qsort(sorted_d, nr_subdirs, sizeof(d), comp_subdir);
> +	for (i = 0; i < nr_subdirs; i++)
> +		list_add_tail(&sorted_d[i]->d_child, &subdirs->i_subdirs);
> +	free(sorted_d);
> +	return 0;
> +}

Nope, no need this function, please fold it in erofs_prepare_dir_file().
Again, please sort the subdirs in erofs_prepare_dir_file().

> +
>  struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  {
>  	int ret;
>  	DIR *_dir;
>  	struct dirent *dp;
>  	struct erofs_dentry *d;
> -	unsigned int nr_subdirs;
> +	struct erofs_subdirs subdirs;
>  
>  	ret = erofs_prepare_xattr_ibody(dir);
>  	if (ret < 0)
> @@ -972,7 +992,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  		return ERR_PTR(-errno);
>  	}
>  
> -	nr_subdirs = 0;
> +	erofs_subdirs_init(dir, &subdirs);
>  	while (1) {
>  		/*
>  		 * set errno to 0 before calling readdir() in order to
> @@ -991,12 +1011,11 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  		if (erofs_is_exclude_path(dir->i_srcpath, dp->d_name))
>  			continue;
>  
> -		d = erofs_d_alloc(dir, dp->d_name);
> +		d = erofs_d_alloc(&subdirs, dp->d_name);
>  		if (IS_ERR(d)) {
>  			ret = PTR_ERR(d);
>  			goto err_closedir;
>  		}
> -		nr_subdirs++;
>  
>  		/* to count i_nlink for directories */
>  		d->type = (dp->d_type == DT_DIR ?
> @@ -1009,7 +1028,11 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  	}
>  	closedir(_dir);
>  
> -	ret = erofs_prepare_dir_file(dir, nr_subdirs);
> +	ret = erofs_subdirs_sorted(&subdirs);
> +	if (ret) goto err;
> +
> +	list_replace(&subdirs.i_subdirs, &dir->i_subdirs);

No need list_replace here.

Thanks,
Gao Xiang

> +	ret = erofs_prepare_dir_file(dir);
>  	if (ret)
>  		goto err;
>  
> -- 
> 2.17.1
> 

Re: [PATCH v1 2/2] erofs-utils: introduce erofs_subdirs to one dir for sort

[Li GuiFu via Linux-erofs]

GaoXiang

On 2021/5/7 15:09, Gao Xiang wrote:
> Hi Guifu,
> 
> On Wed, May 05, 2021 at 10:26:15PM +0800, Li Guifu via Linux-erofs wrote:
>> The structure erofs_subdirs has a dir number and a list_head,
>> the list_head is the same with i_subdirs in the inode.
>> Using erofs_subdirs as a temp place for dentrys under the dir,
>> and then sort it before replace to i_subdirs
>>
>> Signed-off-by: Li Guifu <bluce.lee@aliyun.com>
>> ---
>>  include/erofs/internal.h |  5 +++
>>  lib/inode.c              | 95 +++++++++++++++++++++++++---------------
>>  2 files changed, 64 insertions(+), 36 deletions(-)
>>
>> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
>> index 1339341..7cd42ca 100644
>> --- a/include/erofs/internal.h
>> +++ b/include/erofs/internal.h
>> @@ -172,6 +172,11 @@ struct erofs_inode {
>>  #endif
>>  };
>>  
>> +struct erofs_subdirs {
>> +	struct list_head i_subdirs;
>> +	unsigned int nr_subdirs;
>> +};
>> +
>>  static inline bool is_inode_layout_compression(struct erofs_inode *inode)
>>  {
>>  	return erofs_inode_is_data_compressed(inode->datalayout);
>> diff --git a/lib/inode.c b/lib/inode.c
>> index 787e5b4..3e138a6 100644
>> --- a/lib/inode.c
>> +++ b/lib/inode.c
>> @@ -96,7 +96,7 @@ unsigned int erofs_iput(struct erofs_inode *inode)
>>  	return 0;
>>  }
>>  
>> -struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent,
>> +struct erofs_dentry *erofs_d_alloc(struct erofs_subdirs *subdirs,
>>  				   const char *name)
>>  {
>>  	struct erofs_dentry *d = malloc(sizeof(*d));
>> @@ -107,7 +107,8 @@ struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent,
>>  	strncpy(d->name, name, EROFS_NAME_LEN - 1);
>>  	d->name[EROFS_NAME_LEN - 1] = '\0';
>>  
>> -	list_add_tail(&d->d_child, &parent->i_subdirs);
>> +	list_add_tail(&d->d_child, &subdirs->i_subdirs);
>> +	subdirs->nr_subdirs++;
>>  	return d;
>>  }
>>  
>> @@ -150,38 +151,12 @@ static int comp_subdir(const void *a, const void *b)
>>  	return strcmp(da->name, db->name);
>>  }
>>  
>> -int erofs_prepare_dir_file(struct erofs_inode *dir, unsigned int nr_subdirs)
>> +int erofs_prepare_dir_file(struct erofs_inode *dir)
> 
> Err... nope, that is not what I suggested, I was suggesting
> 
> int erofs_prepare_dir_file(struct erofs_inode *dir,
> 			   struct erofs_subdirs *subdirs)
> 
>>  {
>> -	struct erofs_dentry *d, *n, **sorted_d;
>> -	unsigned int d_size, i_nlink, i;
>> +	struct erofs_dentry *d;
>> +	unsigned int d_size, i_nlink;
>>  	int ret;
>>  
>> -	/* dot is pointed to the current dir inode */
>> -	d = erofs_d_alloc(dir, ".");
>> -	d->inode = erofs_igrab(dir);
>> -	d->type = EROFS_FT_DIR;
>> -
>> -	/* dotdot is pointed to the parent dir */
>> -	d = erofs_d_alloc(dir, "..");
>> -	d->inode = erofs_igrab(dir->i_parent);
>> -	d->type = EROFS_FT_DIR;
> 
> Leave these two here, since it's a part of dir preparation.
I think  this erofs_prepare_dir_file() funtion just do space prepare
that  dirs have been sorted before

. and .. should been added along with readir

Re: [PATCH v1 2/2] erofs-utils: introduce erofs_subdirs to one dir for sort

[Gao Xiang]

On Sun, May 09, 2021 at 10:30:06PM +0800, Li GuiFu wrote:
> GaoXiang
> 
> On 2021/5/7 15:09, Gao Xiang wrote:
> > Hi Guifu,
> > 
> > On Wed, May 05, 2021 at 10:26:15PM +0800, Li Guifu via Linux-erofs wrote:
> >> The structure erofs_subdirs has a dir number and a list_head,
> >> the list_head is the same with i_subdirs in the inode.
> >> Using erofs_subdirs as a temp place for dentrys under the dir,
> >> and then sort it before replace to i_subdirs
> >>
> >> Signed-off-by: Li Guifu <bluce.lee@aliyun.com>
> >> ---
> >>  include/erofs/internal.h |  5 +++
> >>  lib/inode.c              | 95 +++++++++++++++++++++++++---------------
> >>  2 files changed, 64 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> >> index 1339341..7cd42ca 100644
> >> --- a/include/erofs/internal.h
> >> +++ b/include/erofs/internal.h
> >> @@ -172,6 +172,11 @@ struct erofs_inode {
> >>  #endif
> >>  };
> >>  
> >> +struct erofs_subdirs {
> >> +	struct list_head i_subdirs;
> >> +	unsigned int nr_subdirs;
> >> +};
> >> +
> >>  static inline bool is_inode_layout_compression(struct erofs_inode *inode)
> >>  {
> >>  	return erofs_inode_is_data_compressed(inode->datalayout);
> >> diff --git a/lib/inode.c b/lib/inode.c
> >> index 787e5b4..3e138a6 100644
> >> --- a/lib/inode.c
> >> +++ b/lib/inode.c
> >> @@ -96,7 +96,7 @@ unsigned int erofs_iput(struct erofs_inode *inode)
> >>  	return 0;
> >>  }
> >>  
> >> -struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent,
> >> +struct erofs_dentry *erofs_d_alloc(struct erofs_subdirs *subdirs,
> >>  				   const char *name)
> >>  {
> >>  	struct erofs_dentry *d = malloc(sizeof(*d));
> >> @@ -107,7 +107,8 @@ struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent,
> >>  	strncpy(d->name, name, EROFS_NAME_LEN - 1);
> >>  	d->name[EROFS_NAME_LEN - 1] = '\0';
> >>  
> >> -	list_add_tail(&d->d_child, &parent->i_subdirs);
> >> +	list_add_tail(&d->d_child, &subdirs->i_subdirs);
> >> +	subdirs->nr_subdirs++;
> >>  	return d;
> >>  }
> >>  
> >> @@ -150,38 +151,12 @@ static int comp_subdir(const void *a, const void *b)
> >>  	return strcmp(da->name, db->name);
> >>  }
> >>  
> >> -int erofs_prepare_dir_file(struct erofs_inode *dir, unsigned int nr_subdirs)
> >> +int erofs_prepare_dir_file(struct erofs_inode *dir)
> > 
> > Err... nope, that is not what I suggested, I was suggesting
> > 
> > int erofs_prepare_dir_file(struct erofs_inode *dir,
> > 			   struct erofs_subdirs *subdirs)
> > 
> >>  {
> >> -	struct erofs_dentry *d, *n, **sorted_d;
> >> -	unsigned int d_size, i_nlink, i;
> >> +	struct erofs_dentry *d;
> >> +	unsigned int d_size, i_nlink;
> >>  	int ret;
> >>  
> >> -	/* dot is pointed to the current dir inode */
> >> -	d = erofs_d_alloc(dir, ".");
> >> -	d->inode = erofs_igrab(dir);
> >> -	d->type = EROFS_FT_DIR;
> >> -
> >> -	/* dotdot is pointed to the parent dir */
> >> -	d = erofs_d_alloc(dir, "..");
> >> -	d->inode = erofs_igrab(dir->i_parent);
> >> -	d->type = EROFS_FT_DIR;
> > 
> > Leave these two here, since it's a part of dir preparation.
> I think  this erofs_prepare_dir_file() funtion just do space prepare
> that  dirs have been sorted before
> 
> . and .. should been added along with readir

Nope, I don't think introduce another two one-shot fixed helpers
would be helpful here.

Also, prepare_dir_file() clearly means "prepare for the dir file".
It can naturally accept all original subdir inputs, add ".", ".."
dirents, sort them all in dir formats and output the dir files.

I don't know what is wrong with such clean sematics? And you must
seperate it into several helpers?

Thanks,
Gao Xiang

> 
>