Re: [PATCH -next] nilfs2: use common implementation of file type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2024/8/14 21:57, Ryusuke Konishi wrote:
On Wed, Aug 14, 2024 at 9:31 PM Huang Xiaojia  wrote:
Deduplicate the nilfs2 file type conversion implementation and
remove NILFS_FT_* definitions since it's the same as defined
by POSIX.

Signed-off-by: Huang Xiaojia <huangxiaojia2@xxxxxxxxxx>
---
  fs/nilfs2/dir.c                    | 44 ++++--------------------------
  include/uapi/linux/nilfs2_ondisk.h | 16 -----------
  2 files changed, 5 insertions(+), 55 deletions(-)

diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 4a29b0138d75..ba6bc6efcf11 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -231,37 +231,6 @@ static struct nilfs_dir_entry *nilfs_next_entry(struct nilfs_dir_entry *p)
                                           nilfs_rec_len_from_disk(p->rec_len));
  }

-static unsigned char
-nilfs_filetype_table[NILFS_FT_MAX] = {
-       [NILFS_FT_UNKNOWN]      = DT_UNKNOWN,
-       [NILFS_FT_REG_FILE]     = DT_REG,
-       [NILFS_FT_DIR]          = DT_DIR,
-       [NILFS_FT_CHRDEV]       = DT_CHR,
-       [NILFS_FT_BLKDEV]       = DT_BLK,
-       [NILFS_FT_FIFO]         = DT_FIFO,
-       [NILFS_FT_SOCK]         = DT_SOCK,
-       [NILFS_FT_SYMLINK]      = DT_LNK,
-};
-
-#define S_SHIFT 12
-static unsigned char
-nilfs_type_by_mode[(S_IFMT >> S_SHIFT) + 1] = {
-       [S_IFREG >> S_SHIFT]    = NILFS_FT_REG_FILE,
-       [S_IFDIR >> S_SHIFT]    = NILFS_FT_DIR,
-       [S_IFCHR >> S_SHIFT]    = NILFS_FT_CHRDEV,
-       [S_IFBLK >> S_SHIFT]    = NILFS_FT_BLKDEV,
-       [S_IFIFO >> S_SHIFT]    = NILFS_FT_FIFO,
-       [S_IFSOCK >> S_SHIFT]   = NILFS_FT_SOCK,
-       [S_IFLNK >> S_SHIFT]    = NILFS_FT_SYMLINK,
-};
-
-static void nilfs_set_de_type(struct nilfs_dir_entry *de, struct inode *inode)
-{
-       umode_t mode = inode->i_mode;
-
-       de->file_type = nilfs_type_by_mode[(mode & S_IFMT)>>S_SHIFT];
-}
-
  static int nilfs_readdir(struct file *file, struct dir_context *ctx)
  {
         loff_t pos = ctx->pos;
@@ -297,10 +266,7 @@ static int nilfs_readdir(struct file *file, struct dir_context *ctx)
                         if (de->inode) {
                                 unsigned char t;

-                               if (de->file_type < NILFS_FT_MAX)
-                                       t = nilfs_filetype_table[de->file_type];
-                               else
-                                       t = DT_UNKNOWN;
+                               t = fs_ftype_to_dtype(de->file_type);

                                 if (!dir_emit(ctx, de->name, de->name_len,
                                                 le64_to_cpu(de->inode), t)) {
@@ -444,7 +410,7 @@ void nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,
         err = nilfs_prepare_chunk(folio, from, to);
         BUG_ON(err);
         de->inode = cpu_to_le64(inode->i_ino);
-       nilfs_set_de_type(de, inode);
+       de->file_type = fs_umode_to_ftype(inode->i_mode);
         nilfs_commit_chunk(folio, mapping, from, to);
         inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
  }
@@ -531,7 +497,7 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
         de->name_len = namelen;
         memcpy(de->name, name, namelen);
         de->inode = cpu_to_le64(inode->i_ino);
-       nilfs_set_de_type(de, inode);
+       de->file_type = fs_umode_to_ftype(inode->i_mode);
         nilfs_commit_chunk(folio, folio->mapping, from, to);
         inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
         nilfs_mark_inode_dirty(dir);
@@ -612,14 +578,14 @@ int nilfs_make_empty(struct inode *inode, struct inode *parent)
         de->rec_len = nilfs_rec_len_to_disk(NILFS_DIR_REC_LEN(1));
         memcpy(de->name, ".\0\0", 4);
         de->inode = cpu_to_le64(inode->i_ino);
-       nilfs_set_de_type(de, inode);
+       de->file_type = fs_umode_to_ftype(inode->i_mode);

         de = (struct nilfs_dir_entry *)(kaddr + NILFS_DIR_REC_LEN(1));
         de->name_len = 2;
         de->rec_len = nilfs_rec_len_to_disk(chunk_size - NILFS_DIR_REC_LEN(1));
         de->inode = cpu_to_le64(parent->i_ino);
         memcpy(de->name, "..\0", 4);
-       nilfs_set_de_type(de, inode);
+       de->file_type = fs_umode_to_ftype(inode->i_mode);
         kunmap_local(kaddr);
         nilfs_commit_chunk(folio, mapping, 0, chunk_size);
  fail:
diff --git a/include/uapi/linux/nilfs2_ondisk.h b/include/uapi/linux/nilfs2_ondisk.h
index c23f91ae5fe8..f52c338103a5 100644
--- a/include/uapi/linux/nilfs2_ondisk.h
+++ b/include/uapi/linux/nilfs2_ondisk.h
@@ -306,22 +306,6 @@ struct nilfs_dir_entry {
         char    pad;
  };

-/*
- * NILFS directory file types.  Only the low 3 bits are used.  The
- * other bits are reserved for now.
- */
-enum {
-       NILFS_FT_UNKNOWN,
-       NILFS_FT_REG_FILE,
-       NILFS_FT_DIR,
-       NILFS_FT_CHRDEV,
-       NILFS_FT_BLKDEV,
-       NILFS_FT_FIFO,
-       NILFS_FT_SOCK,
-       NILFS_FT_SYMLINK,
-       NILFS_FT_MAX
-};
-
  /*
   * NILFS_DIR_PAD defines the directory entries boundaries
   *
--
2.34.1

Hi Huang Xiaojia,

I understand the intention of using common code for conversion between
ftype and dtype. I haven't fully reviewed this yet, including its
compatibility, but I agree with the direction.

However, please do not remove the definitions of NILFS_FT_* in
nilfs2_ondisk.h (even if there is no longer a reference from the
kernel implementation).

These are identifiers exposed to user space tools as a part of the
disk format definition in the uapi header, and removing them means
changing the user interface.

In the future, if FT_* is exposed to userspace, I think we will
redefine them as aliases to them. For now, leave them as they are.
Even if they should be unified to FT_*, they should be considered to
be phased out rather than suddenly removed.

Thanks,
Ryusuke Konishi

Hi Ryusuke Konishi,

Thanks for your reply. Sorry for my negelect over user space tools.
I will keep NILFS_FT_* in the following patch.

Best Regards,
Huang Xiaojia





[Index of Archives]     [Linux Filesystem Development]     [Linux BTRFS]     [Linux CIFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux