Hi Gao, I found that in the loop erofs_iget_from_srcpath() is called in different order due to readdir and erofs_iget_from_srcpath() calls erofs_new_inode() which fills i_ino[0] for newly created inode. I think this i_ino[0] having different values caused the difference in the output.
On Tue, Dec 3, 2024 at 10:38 AM Gao Xiang <hsiang...@linux.alibaba.com> wrote: > > Hi Jooyung, > > On 2024/12/3 08:27, Jooyung Han wrote: > > The iteration order of opendir/readdir depends on filesystem > > implementation. Hence, even with the same contents, the filesystem of > > the input directory affects the output. > > > > In this change, opendir/readdir is replaced with scandir for stable > > order of iteration. This produces the same output regardless of the > > filesystem of the input directory. > > > > Test: mkfs.erofs ... inputdir(ext3) > > Test: mkfs.erofs ... inputdir(tmpfs) > > # should generate the same output > > Signed-off-by: Jooyung Han <jooy...@google.com> > > Thanks for the patch. I haven't tested the current behavior (1.8.2), > but EROFS will sort all dirents in a directory in > erofs_prepare_dir_file(). And then dump all subdirs in > erofs_mkfs_dump_tree(). > > It seems both dirents and inodes are sorted in the alphabetical > order, could you give more hints about this? > > Thanks, > Gao Xiang > > > --- > > > > v1: > > https://lore.kernel.org/linux-erofs/20241202232620.3604736-1-jooy...@google.com/ > > change since v1: > > - modify commit msg (no change-id/bug/typo) > > - rename the label to err_cleanup > > > > lib/inode.c | 39 ++++++++++++++------------------------- > > 1 file changed, 14 insertions(+), 25 deletions(-) > > > > diff --git a/lib/inode.c b/lib/inode.c > > index f553bec..097deef 100644 > > --- a/lib/inode.c > > +++ b/lib/inode.c > > @@ -1422,37 +1422,25 @@ static void erofs_mkfs_flushjobs(struct > > erofs_sb_info *sbi) > > static int erofs_mkfs_handle_directory(struct erofs_inode *dir) > > { > > struct erofs_sb_info *sbi = dir->sbi; > > - DIR *_dir; > > - struct dirent *dp; > > + struct dirent *dp, **dent; > > + int i, num_entries; > > struct erofs_dentry *d; > > unsigned int nr_subdirs, i_nlink; > > int ret; > > > > - _dir = opendir(dir->i_srcpath); > > - if (!_dir) { > > - erofs_err("failed to opendir at %s: %s", > > + num_entries = scandir(dir->i_srcpath, &dent, NULL, alphasort); > > + if (num_entries == -1) { > > + erofs_err("failed to scandir at %s: %s", > > dir->i_srcpath, erofs_strerror(-errno)); > > return -errno; > > } > > > > nr_subdirs = 0; > > i_nlink = 0; > > - while (1) { > > + for (i = 0; i < num_entries; free(dent[i]), i++) { > > char buf[PATH_MAX]; > > struct erofs_inode *inode; > > - > > - /* > > - * set errno to 0 before calling readdir() in order to > > - * distinguish end of stream and from an error. > > - */ > > - errno = 0; > > - dp = readdir(_dir); > > - if (!dp) { > > - if (!errno) > > - break; > > - ret = -errno; > > - goto err_closedir; > > - } > > + dp = dent[i]; > > > > if (is_dot_dotdot(dp->d_name)) { > > ++i_nlink; > > @@ -1466,17 +1454,17 @@ static int erofs_mkfs_handle_directory(struct > > erofs_inode *dir) > > d = erofs_d_alloc(dir, dp->d_name); > > if (IS_ERR(d)) { > > ret = PTR_ERR(d); > > - goto err_closedir; > > + goto err_cleanup; > > } > > > > ret = snprintf(buf, PATH_MAX, "%s/%s", dir->i_srcpath, > > d->name); > > if (ret < 0 || ret >= PATH_MAX) > > - goto err_closedir; > > + goto err_cleanup; > > > > inode = erofs_iget_from_srcpath(sbi, buf); > > if (IS_ERR(inode)) { > > ret = PTR_ERR(inode); > > - goto err_closedir; > > + goto err_cleanup; > > } > > d->inode = inode; > > d->type = erofs_mode_to_ftype(inode->i_mode); > > @@ -1484,7 +1472,7 @@ static int erofs_mkfs_handle_directory(struct > > erofs_inode *dir) > > erofs_dbg("file %s added (type %u)", buf, d->type); > > nr_subdirs++; > > } > > - closedir(_dir); > > + free(dent); > > > > ret = erofs_init_empty_dir(dir); > > if (ret) > > @@ -1506,8 +1494,9 @@ static int erofs_mkfs_handle_directory(struct > > erofs_inode *dir) > > > > return erofs_mkfs_go(sbi, EROFS_MKFS_JOB_DIR, &dir, sizeof(dir)); > > > > -err_closedir: > > - closedir(_dir); > > +err_cleanup: > > + for (; i < num_entries; free(dent[i]), i++); > > + free(dent); > > return ret; > > } > > >
