* [PATCH -next] nilfs2: use common implementation of file type @ 2024-08-14 12:38 Huang Xiaojia 2024-08-14 13:57 ` Ryusuke Konishi 0 siblings, 1 reply; 3+ messages in thread From: Huang Xiaojia @ 2024-08-14 12:38 UTC (permalink / raw) To: konishi.ryusuke, yuehaibing; +Cc: linux-nilfs, huangxiaojia2 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@huawei.com> --- 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH -next] nilfs2: use common implementation of file type 2024-08-14 12:38 [PATCH -next] nilfs2: use common implementation of file type Huang Xiaojia @ 2024-08-14 13:57 ` Ryusuke Konishi 2024-08-15 1:17 ` Huang Xiaojia 0 siblings, 1 reply; 3+ messages in thread From: Ryusuke Konishi @ 2024-08-14 13:57 UTC (permalink / raw) To: Huang Xiaojia; +Cc: yuehaibing, linux-nilfs 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@huawei.com> > --- > 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH -next] nilfs2: use common implementation of file type 2024-08-14 13:57 ` Ryusuke Konishi @ 2024-08-15 1:17 ` Huang Xiaojia 0 siblings, 0 replies; 3+ messages in thread From: Huang Xiaojia @ 2024-08-15 1:17 UTC (permalink / raw) To: Ryusuke Konishi; +Cc: yuehaibing, linux-nilfs 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@huawei.com> >> --- >> 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-15 1:17 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-14 12:38 [PATCH -next] nilfs2: use common implementation of file type Huang Xiaojia 2024-08-14 13:57 ` Ryusuke Konishi 2024-08-15 1:17 ` Huang Xiaojia
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox