Re: [PATCH v8 07/24] erofs: add directory operations

2019-08-15 Thread Gao Xiang
Hi Linus,

On Thu, Aug 15, 2019 at 09:13:19AM -0700, Linus Torvalds wrote:
> On Wed, Aug 14, 2019 at 9:42 PM Gao Xiang  wrote:
> >
> > +
> > +static const unsigned char erofs_filetype_table[EROFS_FT_MAX] = {
> > +   [EROFS_FT_UNKNOWN]  = DT_UNKNOWN,
> > +   [EROFS_FT_REG_FILE] = DT_REG,
> > +   [EROFS_FT_DIR]  = DT_DIR,
> > +   [EROFS_FT_CHRDEV]   = DT_CHR,
> > +   [EROFS_FT_BLKDEV]   = DT_BLK,
> > +   [EROFS_FT_FIFO] = DT_FIFO,
> > +   [EROFS_FT_SOCK] = DT_SOCK,
> > +   [EROFS_FT_SYMLINK]  = DT_LNK,
> > +};
> 
> Hmm.
> 
> The EROFS_FT_XYZ values seem to match the normal FT_XYZ values, and
> we've lately tried to just have filesystems use the standard ones
> instead of having a (pointless) duplicate conversion between the two.
> 
> And then you can use the common "fs_ftype_to_dtype()" to convert from
> FT_XYZ to DT_XYZ.
> 
> Maybe I'm missing something, and the EROFS_FT_x list actually differs
> from the normal FT_x list some way, but it would be good to not
> introduce another case of this in normal filesystems, just as we've
> been getting rid of them.
> 
> See for example commit e10892189428 ("ext2: use common file type conversion").

Yes, you're right. There is nothing different with DT_XYZ since
I followed what f2fs did when I wrote this place.

Actually, I noticed that patchset once in mailing list months ago
https://lore.kernel.org/r/20181023201952.GA15676@pathfinder/
but I didn't keep eyes on it (whether this patchset is merged or not...)

OK, let me fix that like other fses. Thanks for pointing out.

Thanks,
Gao Xiang

> 
>Linus


Re: [PATCH v8 07/24] erofs: add directory operations

2019-08-15 Thread Linus Torvalds
On Wed, Aug 14, 2019 at 9:42 PM Gao Xiang  wrote:
>
> +
> +static const unsigned char erofs_filetype_table[EROFS_FT_MAX] = {
> +   [EROFS_FT_UNKNOWN]  = DT_UNKNOWN,
> +   [EROFS_FT_REG_FILE] = DT_REG,
> +   [EROFS_FT_DIR]  = DT_DIR,
> +   [EROFS_FT_CHRDEV]   = DT_CHR,
> +   [EROFS_FT_BLKDEV]   = DT_BLK,
> +   [EROFS_FT_FIFO] = DT_FIFO,
> +   [EROFS_FT_SOCK] = DT_SOCK,
> +   [EROFS_FT_SYMLINK]  = DT_LNK,
> +};

Hmm.

The EROFS_FT_XYZ values seem to match the normal FT_XYZ values, and
we've lately tried to just have filesystems use the standard ones
instead of having a (pointless) duplicate conversion between the two.

And then you can use the common "fs_ftype_to_dtype()" to convert from
FT_XYZ to DT_XYZ.

Maybe I'm missing something, and the EROFS_FT_x list actually differs
from the normal FT_x list some way, but it would be good to not
introduce another case of this in normal filesystems, just as we've
been getting rid of them.

See for example commit e10892189428 ("ext2: use common file type conversion").

   Linus


[PATCH v8 07/24] erofs: add directory operations

2019-08-14 Thread Gao Xiang
This adds functions for directory, mainly readdir.

Signed-off-by: Gao Xiang 
---
 fs/erofs/dir.c | 148 +
 1 file changed, 148 insertions(+)
 create mode 100644 fs/erofs/dir.c

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
new file mode 100644
index ..c52d27bedff4
--- /dev/null
+++ b/fs/erofs/dir.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * linux/fs/erofs/dir.c
+ *
+ * Copyright (C) 2017-2018 HUAWEI, Inc.
+ * http://www.huawei.com/
+ * Created by Gao Xiang 
+ */
+#include "internal.h"
+
+static const unsigned char erofs_filetype_table[EROFS_FT_MAX] = {
+   [EROFS_FT_UNKNOWN]  = DT_UNKNOWN,
+   [EROFS_FT_REG_FILE] = DT_REG,
+   [EROFS_FT_DIR]  = DT_DIR,
+   [EROFS_FT_CHRDEV]   = DT_CHR,
+   [EROFS_FT_BLKDEV]   = DT_BLK,
+   [EROFS_FT_FIFO] = DT_FIFO,
+   [EROFS_FT_SOCK] = DT_SOCK,
+   [EROFS_FT_SYMLINK]  = DT_LNK,
+};
+
+static void debug_one_dentry(unsigned char d_type, const char *de_name,
+unsigned int de_namelen)
+{
+#ifdef CONFIG_EROFS_FS_DEBUG
+   /* since the on-disk name could not have the trailing '\0' */
+   unsigned char dbg_namebuf[EROFS_NAME_LEN + 1];
+
+   memcpy(dbg_namebuf, de_name, de_namelen);
+   dbg_namebuf[de_namelen] = '\0';
+
+   debugln("found dirent %s de_len %u d_type %d", dbg_namebuf,
+   de_namelen, d_type);
+#endif
+}
+
+static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
+  void *dentry_blk, unsigned int *ofs,
+  unsigned int nameoff, unsigned int maxsize)
+{
+   struct erofs_dirent *de = dentry_blk + *ofs;
+   const struct erofs_dirent *end = dentry_blk + nameoff;
+
+   while (de < end) {
+   const char *de_name;
+   unsigned int de_namelen;
+   unsigned char d_type;
+
+   if (de->file_type < EROFS_FT_MAX)
+   d_type = erofs_filetype_table[de->file_type];
+   else
+   d_type = DT_UNKNOWN;
+
+   nameoff = le16_to_cpu(de->nameoff);
+   de_name = (char *)dentry_blk + nameoff;
+
+   /* the last dirent in the block? */
+   if (de + 1 >= end)
+   de_namelen = strnlen(de_name, maxsize - nameoff);
+   else
+   de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
+
+   /* a corrupted entry is found */
+   if (unlikely(nameoff + de_namelen > maxsize ||
+de_namelen > EROFS_NAME_LEN)) {
+   errln("bogus dirent @ nid %llu", EROFS_V(dir)->nid);
+   DBG_BUGON(1);
+   return -EFSCORRUPTED;
+   }
+
+   debug_one_dentry(d_type, de_name, de_namelen);
+   if (!dir_emit(ctx, de_name, de_namelen,
+ le64_to_cpu(de->nid), d_type))
+   /* stopped by some reason */
+   return 1;
+   ++de;
+   *ofs += sizeof(struct erofs_dirent);
+   }
+   *ofs = maxsize;
+   return 0;
+}
+
+static int erofs_readdir(struct file *f, struct dir_context *ctx)
+{
+   struct inode *dir = file_inode(f);
+   struct address_space *mapping = dir->i_mapping;
+   const size_t dirsize = i_size_read(dir);
+   unsigned int i = ctx->pos / EROFS_BLKSIZ;
+   unsigned int ofs = ctx->pos % EROFS_BLKSIZ;
+   int err = 0;
+   bool initial = true;
+
+   while (ctx->pos < dirsize) {
+   struct page *dentry_page;
+   struct erofs_dirent *de;
+   unsigned int nameoff, maxsize;
+
+   dentry_page = read_mapping_page(mapping, i, NULL);
+   if (IS_ERR(dentry_page))
+   continue;
+
+   de = (struct erofs_dirent *)kmap(dentry_page);
+
+   nameoff = le16_to_cpu(de->nameoff);
+
+   if (unlikely(nameoff < sizeof(struct erofs_dirent) ||
+nameoff >= PAGE_SIZE)) {
+   errln("%s, invalid de[0].nameoff %u @ nid %llu",
+ __func__, nameoff, EROFS_V(dir)->nid);
+   err = -EFSCORRUPTED;
+   goto skip_this;
+   }
+
+   maxsize = min_t(unsigned int,
+   dirsize - ctx->pos + ofs, PAGE_SIZE);
+
+   /* search dirents at the arbitrary position */
+   if (unlikely(initial)) {
+   initial = false;
+
+   ofs = roundup(ofs, sizeof(struct erofs_dirent));
+   if (unlikely(ofs >= nameoff))
+   goto skip_this;
+   }
+
+   err = erofs_fill_dentries(dir, ctx, de, ,
+