* [RFC][PATCH v2 10/10] btrfs: use common file type conversion @ 2018-10-23 21:17 ` Phillip Potter 2018-10-24 10:11 ` David Sterba 0 siblings, 1 reply; 3+ messages in thread From: Phillip Potter @ 2018-10-23 21:17 UTC (permalink / raw) To: clm Cc: linux-kernel, amir73il, viro, jbacik, dsterba, linux-fsdevel, linux-btrfs Deduplicate the btrfs file type conversion implementation. Original patch by Amir Goldstein. v2: - Rebased against Linux 4.19 by Phillip Potter - Compile-time checks added by Phillip Potter to make sure the BTRFS_FT_x enum values stay same as FT_x values v1: - Initial implementation Signed-off-by: Phillip Potter <phil@philpotter.co.uk> --- fs/btrfs/btrfs_inode.h | 2 -- fs/btrfs/delayed-inode.c | 2 +- fs/btrfs/inode.c | 35 +++++++++++++++++---------------- include/uapi/linux/btrfs_tree.h | 2 ++ 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h index 1343ac57b438..c7c6db6b4a35 100644 --- a/fs/btrfs/btrfs_inode.h +++ b/fs/btrfs/btrfs_inode.h @@ -196,8 +196,6 @@ struct btrfs_inode { struct inode vfs_inode; }; -extern unsigned char btrfs_filetype_table[]; - static inline struct btrfs_inode *BTRFS_I(const struct inode *inode) { return container_of(inode, struct btrfs_inode, vfs_inode); diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index f51b509f2d9b..c1da34e3a775 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -1691,7 +1691,7 @@ int btrfs_readdir_delayed_dir_index(struct dir_context *ctx, name = (char *)(di + 1); name_len = btrfs_stack_dir_name_len(di); - d_type = btrfs_filetype_table[di->type]; + d_type = fs_dtype(di->type); btrfs_disk_key_to_cpu(&location, &di->location); over = !dir_emit(ctx, name, name_len, diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 3ea5339603cf..4cbdcba4799a 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -73,17 +73,6 @@ struct kmem_cache *btrfs_trans_handle_cachep; struct kmem_cache *btrfs_path_cachep; struct kmem_cache *btrfs_free_space_cachep; -#define S_SHIFT 12 -static const unsigned char btrfs_type_by_mode[S_IFMT >> S_SHIFT] = { - [S_IFREG >> S_SHIFT] = BTRFS_FT_REG_FILE, - [S_IFDIR >> S_SHIFT] = BTRFS_FT_DIR, - [S_IFCHR >> S_SHIFT] = BTRFS_FT_CHRDEV, - [S_IFBLK >> S_SHIFT] = BTRFS_FT_BLKDEV, - [S_IFIFO >> S_SHIFT] = BTRFS_FT_FIFO, - [S_IFSOCK >> S_SHIFT] = BTRFS_FT_SOCK, - [S_IFLNK >> S_SHIFT] = BTRFS_FT_SYMLINK, -}; - static int btrfs_setsize(struct inode *inode, struct iattr *attr); static int btrfs_truncate(struct inode *inode, bool skip_writeback); static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent); @@ -5803,10 +5792,6 @@ static struct dentry *btrfs_lookup(struct inode *dir, struct dentry *dentry, return d_splice_alias(inode, dentry); } -unsigned char btrfs_filetype_table[] = { - DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK -}; - /* * All this infrastructure exists because dir_emit can fault, and we are holding * the tree lock when doing readdir. For now just allocate a buffer and copy @@ -5945,7 +5930,23 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx) name_ptr = (char *)(entry + 1); read_extent_buffer(leaf, name_ptr, (unsigned long)(di + 1), name_len); - put_unaligned(btrfs_filetype_table[btrfs_dir_type(leaf, di)], + + /* + * compile-time asserts that generic FT_x types + * still match BTRFS_FT_x types - no need to list + * in other functions as well as build will + * fail either way + */ + BUILD_BUG_ON(BTRFS_FT_UNKNOWN != FT_UNKNOWN); + BUILD_BUG_ON(BTRFS_FT_REG_FILE != FT_REG_FILE); + BUILD_BUG_ON(BTRFS_FT_DIR != FT_DIR); + BUILD_BUG_ON(BTRFS_FT_CHRDEV != FT_CHRDEV); + BUILD_BUG_ON(BTRFS_FT_BLKDEV != FT_BLKDEV); + BUILD_BUG_ON(BTRFS_FT_FIFO != FT_FIFO); + BUILD_BUG_ON(BTRFS_FT_SOCK != FT_SOCK); + BUILD_BUG_ON(BTRFS_FT_SYMLINK != FT_SYMLINK); + + put_unaligned(fs_dtype(btrfs_dir_type(leaf, di)), &entry->type); btrfs_dir_item_key_to_cpu(leaf, di, &location); put_unaligned(location.objectid, &entry->ino); @@ -6350,7 +6351,7 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans, static inline u8 btrfs_inode_type(struct inode *inode) { - return btrfs_type_by_mode[(inode->i_mode & S_IFMT) >> S_SHIFT]; + return fs_umode_to_ftype(inode->i_mode); } /* diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h index aff1356c2bb8..5747cffa09fa 100644 --- a/include/uapi/linux/btrfs_tree.h +++ b/include/uapi/linux/btrfs_tree.h @@ -307,6 +307,8 @@ * * Used by: * struct btrfs_dir_item.type + * + * Values 0..7 should match common file type values in file_type.h. */ #define BTRFS_FT_UNKNOWN 0 #define BTRFS_FT_REG_FILE 1 -- 2.17.2 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC][PATCH v2 10/10] btrfs: use common file type conversion 2018-10-23 21:17 ` [RFC][PATCH v2 10/10] btrfs: use common file type conversion Phillip Potter @ 2018-10-24 10:11 ` David Sterba 2018-10-24 13:28 ` Phillip Potter 0 siblings, 1 reply; 3+ messages in thread From: David Sterba @ 2018-10-24 10:11 UTC (permalink / raw) To: Phillip Potter Cc: clm, linux-kernel, amir73il, viro, jbacik, dsterba, linux-fsdevel, linux-btrfs On Tue, Oct 23, 2018 at 10:17:28PM +0100, Phillip Potter wrote: > Deduplicate the btrfs file type conversion implementation. The per-filesystem changelogs need a brief explanation why this is done, like "Filesystems that use the same filetypes as defined by POSIX do not need to define their own versions and use the common defines in file_type.h", rephrase at you like. > Original patch by Amir Goldstein. > > v2: > - Rebased against Linux 4.19 by Phillip Potter > - Compile-time checks added by Phillip Potter to make > sure the BTRFS_FT_x enum values stay same as FT_x values > > v1: > - Initial implementation > > Signed-off-by: Phillip Potter <phil@philpotter.co.uk> > --- > fs/btrfs/btrfs_inode.h | 2 -- > fs/btrfs/delayed-inode.c | 2 +- > fs/btrfs/inode.c | 35 +++++++++++++++++---------------- > include/uapi/linux/btrfs_tree.h | 2 ++ > 4 files changed, 21 insertions(+), 20 deletions(-) > > diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h > index 1343ac57b438..c7c6db6b4a35 100644 > --- a/fs/btrfs/btrfs_inode.h > +++ b/fs/btrfs/btrfs_inode.h > @@ -196,8 +196,6 @@ struct btrfs_inode { > struct inode vfs_inode; > }; > > -extern unsigned char btrfs_filetype_table[]; > - > static inline struct btrfs_inode *BTRFS_I(const struct inode *inode) > { > return container_of(inode, struct btrfs_inode, vfs_inode); > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c > index f51b509f2d9b..c1da34e3a775 100644 > --- a/fs/btrfs/delayed-inode.c > +++ b/fs/btrfs/delayed-inode.c > @@ -1691,7 +1691,7 @@ int btrfs_readdir_delayed_dir_index(struct dir_context *ctx, > name = (char *)(di + 1); > name_len = btrfs_stack_dir_name_len(di); > > - d_type = btrfs_filetype_table[di->type]; > + d_type = fs_dtype(di->type); > btrfs_disk_key_to_cpu(&location, &di->location); > > over = !dir_emit(ctx, name, name_len, > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 3ea5339603cf..4cbdcba4799a 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -73,17 +73,6 @@ struct kmem_cache *btrfs_trans_handle_cachep; > struct kmem_cache *btrfs_path_cachep; > struct kmem_cache *btrfs_free_space_cachep; > > -#define S_SHIFT 12 > -static const unsigned char btrfs_type_by_mode[S_IFMT >> S_SHIFT] = { > - [S_IFREG >> S_SHIFT] = BTRFS_FT_REG_FILE, > - [S_IFDIR >> S_SHIFT] = BTRFS_FT_DIR, > - [S_IFCHR >> S_SHIFT] = BTRFS_FT_CHRDEV, > - [S_IFBLK >> S_SHIFT] = BTRFS_FT_BLKDEV, > - [S_IFIFO >> S_SHIFT] = BTRFS_FT_FIFO, > - [S_IFSOCK >> S_SHIFT] = BTRFS_FT_SOCK, > - [S_IFLNK >> S_SHIFT] = BTRFS_FT_SYMLINK, > -}; > - > static int btrfs_setsize(struct inode *inode, struct iattr *attr); > static int btrfs_truncate(struct inode *inode, bool skip_writeback); > static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent); > @@ -5803,10 +5792,6 @@ static struct dentry *btrfs_lookup(struct inode *dir, struct dentry *dentry, > return d_splice_alias(inode, dentry); > } > > -unsigned char btrfs_filetype_table[] = { > - DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK > -}; > - > /* > * All this infrastructure exists because dir_emit can fault, and we are holding > * the tree lock when doing readdir. For now just allocate a buffer and copy > @@ -5945,7 +5930,23 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx) > name_ptr = (char *)(entry + 1); > read_extent_buffer(leaf, name_ptr, (unsigned long)(di + 1), > name_len); > - put_unaligned(btrfs_filetype_table[btrfs_dir_type(leaf, di)], > + > + /* > + * compile-time asserts that generic FT_x types > + * still match BTRFS_FT_x types - no need to list > + * in other functions as well as build will > + * fail either way > + */ Please format comments to 80 columns. > + BUILD_BUG_ON(BTRFS_FT_UNKNOWN != FT_UNKNOWN); > + BUILD_BUG_ON(BTRFS_FT_REG_FILE != FT_REG_FILE); > + BUILD_BUG_ON(BTRFS_FT_DIR != FT_DIR); > + BUILD_BUG_ON(BTRFS_FT_CHRDEV != FT_CHRDEV); > + BUILD_BUG_ON(BTRFS_FT_BLKDEV != FT_BLKDEV); > + BUILD_BUG_ON(BTRFS_FT_FIFO != FT_FIFO); > + BUILD_BUG_ON(BTRFS_FT_SOCK != FT_SOCK); > + BUILD_BUG_ON(BTRFS_FT_SYMLINK != FT_SYMLINK); > + > + put_unaligned(fs_dtype(btrfs_dir_type(leaf, di)), > &entry->type); > btrfs_dir_item_key_to_cpu(leaf, di, &location); > put_unaligned(location.objectid, &entry->ino); > @@ -6350,7 +6351,7 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans, > > static inline u8 btrfs_inode_type(struct inode *inode) > { > - return btrfs_type_by_mode[(inode->i_mode & S_IFMT) >> S_SHIFT]; > + return fs_umode_to_ftype(inode->i_mode); > } > > /* > diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h > index aff1356c2bb8..5747cffa09fa 100644 > --- a/include/uapi/linux/btrfs_tree.h > +++ b/include/uapi/linux/btrfs_tree.h > @@ -307,6 +307,8 @@ > * > * Used by: > * struct btrfs_dir_item.type > + * > + * Values 0..7 should match common file type values in file_type.h. I think it's rather 'must' than 'should' as there are the compile-time checks. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC][PATCH v2 10/10] btrfs: use common file type conversion 2018-10-24 10:11 ` David Sterba @ 2018-10-24 13:28 ` Phillip Potter 0 siblings, 0 replies; 3+ messages in thread From: Phillip Potter @ 2018-10-24 13:28 UTC (permalink / raw) To: David Sterba Cc: clm, amir73il, viro, jbacik, dsterba, linux-fsdevel, linux-btrfs On Wed, Oct 24, 2018 at 12:11:28PM +0200, David Sterba wrote: > On Tue, Oct 23, 2018 at 10:17:28PM +0100, Phillip Potter wrote: > > Deduplicate the btrfs file type conversion implementation. > > The per-filesystem changelogs need a brief explanation why this is done, > like "Filesystems that use the same filetypes as defined by POSIX do not > need to define their own versions and use the common defines in > file_type.h", rephrase at you like. > > > Original patch by Amir Goldstein. > > > > v2: > > - Rebased against Linux 4.19 by Phillip Potter > > - Compile-time checks added by Phillip Potter to make > > sure the BTRFS_FT_x enum values stay same as FT_x values > > > > v1: > > - Initial implementation > > > > Signed-off-by: Phillip Potter <phil@philpotter.co.uk> > > --- > > fs/btrfs/btrfs_inode.h | 2 -- > > fs/btrfs/delayed-inode.c | 2 +- > > fs/btrfs/inode.c | 35 +++++++++++++++++---------------- > > include/uapi/linux/btrfs_tree.h | 2 ++ > > 4 files changed, 21 insertions(+), 20 deletions(-) > > > > diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h > > index 1343ac57b438..c7c6db6b4a35 100644 > > --- a/fs/btrfs/btrfs_inode.h > > +++ b/fs/btrfs/btrfs_inode.h > > @@ -196,8 +196,6 @@ struct btrfs_inode { > > struct inode vfs_inode; > > }; > > > > -extern unsigned char btrfs_filetype_table[]; > > - > > static inline struct btrfs_inode *BTRFS_I(const struct inode *inode) > > { > > return container_of(inode, struct btrfs_inode, vfs_inode); > > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c > > index f51b509f2d9b..c1da34e3a775 100644 > > --- a/fs/btrfs/delayed-inode.c > > +++ b/fs/btrfs/delayed-inode.c > > @@ -1691,7 +1691,7 @@ int btrfs_readdir_delayed_dir_index(struct dir_context *ctx, > > name = (char *)(di + 1); > > name_len = btrfs_stack_dir_name_len(di); > > > > - d_type = btrfs_filetype_table[di->type]; > > + d_type = fs_dtype(di->type); > > btrfs_disk_key_to_cpu(&location, &di->location); > > > > over = !dir_emit(ctx, name, name_len, > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > > index 3ea5339603cf..4cbdcba4799a 100644 > > --- a/fs/btrfs/inode.c > > +++ b/fs/btrfs/inode.c > > @@ -73,17 +73,6 @@ struct kmem_cache *btrfs_trans_handle_cachep; > > struct kmem_cache *btrfs_path_cachep; > > struct kmem_cache *btrfs_free_space_cachep; > > > > -#define S_SHIFT 12 > > -static const unsigned char btrfs_type_by_mode[S_IFMT >> S_SHIFT] = { > > - [S_IFREG >> S_SHIFT] = BTRFS_FT_REG_FILE, > > - [S_IFDIR >> S_SHIFT] = BTRFS_FT_DIR, > > - [S_IFCHR >> S_SHIFT] = BTRFS_FT_CHRDEV, > > - [S_IFBLK >> S_SHIFT] = BTRFS_FT_BLKDEV, > > - [S_IFIFO >> S_SHIFT] = BTRFS_FT_FIFO, > > - [S_IFSOCK >> S_SHIFT] = BTRFS_FT_SOCK, > > - [S_IFLNK >> S_SHIFT] = BTRFS_FT_SYMLINK, > > -}; > > - > > static int btrfs_setsize(struct inode *inode, struct iattr *attr); > > static int btrfs_truncate(struct inode *inode, bool skip_writeback); > > static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent); > > @@ -5803,10 +5792,6 @@ static struct dentry *btrfs_lookup(struct inode *dir, struct dentry *dentry, > > return d_splice_alias(inode, dentry); > > } > > > > -unsigned char btrfs_filetype_table[] = { > > - DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK > > -}; > > - > > /* > > * All this infrastructure exists because dir_emit can fault, and we are holding > > * the tree lock when doing readdir. For now just allocate a buffer and copy > > @@ -5945,7 +5930,23 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx) > > name_ptr = (char *)(entry + 1); > > read_extent_buffer(leaf, name_ptr, (unsigned long)(di + 1), > > name_len); > > - put_unaligned(btrfs_filetype_table[btrfs_dir_type(leaf, di)], > > + > > + /* > > + * compile-time asserts that generic FT_x types > > + * still match BTRFS_FT_x types - no need to list > > + * in other functions as well as build will > > + * fail either way > > + */ > > Please format comments to 80 columns. > > > + BUILD_BUG_ON(BTRFS_FT_UNKNOWN != FT_UNKNOWN); > > + BUILD_BUG_ON(BTRFS_FT_REG_FILE != FT_REG_FILE); > > + BUILD_BUG_ON(BTRFS_FT_DIR != FT_DIR); > > + BUILD_BUG_ON(BTRFS_FT_CHRDEV != FT_CHRDEV); > > + BUILD_BUG_ON(BTRFS_FT_BLKDEV != FT_BLKDEV); > > + BUILD_BUG_ON(BTRFS_FT_FIFO != FT_FIFO); > > + BUILD_BUG_ON(BTRFS_FT_SOCK != FT_SOCK); > > + BUILD_BUG_ON(BTRFS_FT_SYMLINK != FT_SYMLINK); > > + > > + put_unaligned(fs_dtype(btrfs_dir_type(leaf, di)), > > &entry->type); > > btrfs_dir_item_key_to_cpu(leaf, di, &location); > > put_unaligned(location.objectid, &entry->ino); > > @@ -6350,7 +6351,7 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans, > > > > static inline u8 btrfs_inode_type(struct inode *inode) > > { > > - return btrfs_type_by_mode[(inode->i_mode & S_IFMT) >> S_SHIFT]; > > + return fs_umode_to_ftype(inode->i_mode); > > } > > > > /* > > diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h > > index aff1356c2bb8..5747cffa09fa 100644 > > --- a/include/uapi/linux/btrfs_tree.h > > +++ b/include/uapi/linux/btrfs_tree.h > > @@ -307,6 +307,8 @@ > > * > > * Used by: > > * struct btrfs_dir_item.type > > + * > > + * Values 0..7 should match common file type values in file_type.h. > > I think it's rather 'must' than 'should' as there are the compile-time > checks. Dear David, Good points, thank you, I will make these changes and republish as part of a new series. Regards, Phil ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-10-24 13:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20181023201952.GA15676@pathfinder>
2018-10-23 21:17 ` [RFC][PATCH v2 10/10] btrfs: use common file type conversion Phillip Potter
2018-10-24 10:11 ` David Sterba
2018-10-24 13:28 ` Phillip Potter
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).