From: Gao Xiang <xiang@kernel.org> To: Li Guifu <bluce.lee@aliyun.com> Cc: linux-erofs@lists.ozlabs.org Subject: Re: [PATCH v1 2/2] erofs-utils: introduce erofs_subdirs to one dir for sort Date: Fri, 7 May 2021 15:09:59 +0800 [thread overview] Message-ID: <20210507070958.GA8929@hsiangkao-HP-ZHAN-66-Pro-G1> (raw) In-Reply-To: <20210505142615.38306-2-bluce.lee@aliyun.com> 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 >
next prev parent reply other threads:[~2021-05-07 7:10 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-05 14:26 [PATCH v1 1/2] erofs-utils: add list_replace from linux kernel for dirs sorted later Li Guifu via Linux-erofs 2021-05-05 14:26 ` [PATCH v1 2/2] erofs-utils: introduce erofs_subdirs to one dir for sort Li Guifu via Linux-erofs 2021-05-07 7:09 ` Gao Xiang [this message] 2021-05-09 14:30 ` Li GuiFu via Linux-erofs 2021-05-10 1:06 ` Gao Xiang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210507070958.GA8929@hsiangkao-HP-ZHAN-66-Pro-G1 \ --to=xiang@kernel.org \ --cc=bluce.lee@aliyun.com \ --cc=linux-erofs@lists.ozlabs.org \ --subject='Re: [PATCH v1 2/2] erofs-utils: introduce erofs_subdirs to one dir for sort' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
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).