linux-erofs.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Gao Xiang <xiang@kernel.org>
To: Li GuiFu <bluce.lee@aliyun.com>, Li Guifu <bluce.liguifu@huawei.com>
Cc: Gao Xiang <xiang@kernel.org>, linux-erofs@lists.ozlabs.org
Subject: Re: [PATCH v1 2/2] erofs-utils: introduce erofs_subdirs to one dir for sort
Date: Mon, 10 May 2021 09:06:54 +0800	[thread overview]
Message-ID: <20210510010653.GA31518@hsiangkao-HP-ZHAN-66-Pro-G1> (raw)
In-Reply-To: <aa2954c6-442b-86e2-5f0e-27dad4026ff1@aliyun.com>

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

> 
> 

      reply	other threads:[~2021-05-10  1:07 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
2021-05-09 14:30     ` Li GuiFu via Linux-erofs
2021-05-10  1:06       ` Gao Xiang [this message]

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=20210510010653.GA31518@hsiangkao-HP-ZHAN-66-Pro-G1 \
    --to=xiang@kernel.org \
    --cc=bluce.lee@aliyun.com \
    --cc=bluce.liguifu@huawei.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).