[RFC][PATCH v3 01/10] fs: common implementation of file type

[RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

Many file systems use a copy&paste implementation
of dirent to on-disk file type conversions.

Create a common implementation to be used by file systems
with some useful conversion helpers to reduce open coded
file type conversions in file system code.

Original patch written by Amir Goldstein.

v3:
- Rebased against Linux 4.19 by Phillip Potter
- Added SPDX tag to new include/linux/file_type.h
- Added include/linux/file_type.h to MAINTAINERS

v2:
- s/DT_MASK/S_DT_MASK to fix redefinition in drivers/scsi/qla2xxx/qla_def.h
- Explicit initialization of fs_dtype_by_ftype[] using [FT_*] = DT_*

v1:
- Initial implementation

Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
---
 MAINTAINERS               |   1 +
 include/linux/file_type.h | 108 ++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h        |  17 +-----
 3 files changed, 110 insertions(+), 16 deletions(-)
 create mode 100644 include/linux/file_type.h

diff --git a/MAINTAINERS b/MAINTAINERS
index b2f710eee67a..8e5b029886e6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5689,6 +5689,7 @@ L:	linux-fsdevel@vger.kernel.org
 S:	Maintained
 F:	fs/*
 F:	include/linux/fs.h
+F:	include/linux/file_type.h
 F:	include/uapi/linux/fs.h
 
 FINTEK F75375S HARDWARE MONITOR AND FAN CONTROLLER DRIVER
diff --git a/include/linux/file_type.h b/include/linux/file_type.h
new file mode 100644
index 000000000000..f015c41ca90c
--- /dev/null
+++ b/include/linux/file_type.h
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_FILE_TYPE_H
+#define _LINUX_FILE_TYPE_H
+
+/*
+ * This is a common implementation of dirent to fs on-disk
+ * file type conversion.  Although the fs on-disk bits are
+ * specific to every file system, in practice, many file systems
+ * use the exact same on-disk format to describe the lower 3
+ * file type bits that represent the 7 POSIX file types.
+ * All those file systems can use this generic code for the
+ * conversions:
+ *  i_mode -> fs on-disk file type (ftype)
+ *  fs on-disk file type (ftype) -> dirent file type (dtype)
+ *  i_mode -> dirent file type (dtype)
+ */
+
+/*
+ * struct dirent file types
+ * exposed to user via getdents(2), readdir(3)
+ *
+ * These match bits 12..15 of stat.st_mode
+ * (ie "(i_mode >> 12) & 15").
+ */
+#define S_DT_SHIFT	12
+#define S_DT(mode)	(((mode) & S_IFMT) >> S_DT_SHIFT)
+#define S_DT_MASK	(S_IFMT >> S_DT_SHIFT)
+
+#define DT_UNKNOWN	0
+#define DT_FIFO		S_DT(S_IFIFO) /* 1 */
+#define DT_CHR		S_DT(S_IFCHR) /* 2 */
+#define DT_DIR		S_DT(S_IFDIR) /* 4 */
+#define DT_BLK		S_DT(S_IFBLK) /* 6 */
+#define DT_REG		S_DT(S_IFREG) /* 8 */
+#define DT_LNK		S_DT(S_IFLNK) /* 10 */
+#define DT_SOCK		S_DT(S_IFSOCK) /* 12 */
+#define DT_WHT		14
+
+#define DT_MAX		(S_DT_MASK + 1) /* 16 */
+
+/*
+ * fs on-disk file types.
+ * Only the low 3 bits are used for the POSIX file types.
+ * Other bits are reserved for fs private use.
+ *
+ * Note that no fs currently stores the whiteout type on-disk,
+ * so whiteout dirents are exposed to user as DT_CHR.
+ */
+#define FT_UNKNOWN	0
+#define FT_REG_FILE	1
+#define FT_DIR		2
+#define FT_CHRDEV	3
+#define FT_BLKDEV	4
+#define FT_FIFO		5
+#define FT_SOCK		6
+#define FT_SYMLINK	7
+
+#define FT_MAX		8
+
+/*
+ * fs on-disk file type to dirent file type conversion
+ */
+static unsigned char fs_dtype_by_ftype[FT_MAX] = {
+	[FT_UNKNOWN]	= DT_UNKNOWN,
+	[FT_REG_FILE]	= DT_REG,
+	[FT_DIR]	= DT_DIR,
+	[FT_CHRDEV]	= DT_CHR,
+	[FT_BLKDEV]	= DT_BLK,
+	[FT_FIFO]	= DT_FIFO,
+	[FT_SOCK]	= DT_SOCK,
+	[FT_SYMLINK]	= DT_LNK
+};
+
+static inline unsigned char fs_dtype(int filetype)
+{
+	if (filetype >= FT_MAX)
+		return DT_UNKNOWN;
+
+	return fs_dtype_by_ftype[filetype];
+}
+
+/*
+ * dirent file type to fs on-disk file type conversion
+ * Values not initialized explicitly are FT_UNKNOWN (0).
+ */
+static unsigned char fs_ftype_by_dtype[DT_MAX] = {
+	[DT_REG]	= FT_REG_FILE,
+	[DT_DIR]	= FT_DIR,
+	[DT_LNK]	= FT_SYMLINK,
+	[DT_CHR]	= FT_CHRDEV,
+	[DT_BLK]	= FT_BLKDEV,
+	[DT_FIFO]	= FT_FIFO,
+	[DT_SOCK]	= FT_SOCK,
+};
+
+/* st_mode to fs on-disk file type conversion */
+static inline unsigned char fs_umode_to_ftype(umode_t mode)
+{
+	return fs_ftype_by_dtype[S_DT(mode)];
+}
+
+/* st_mode to dirent file type conversion */
+static inline unsigned char fs_umode_to_dtype(umode_t mode)
+{
+	return fs_dtype(fs_umode_to_ftype(mode));
+}
+
+#endif
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 897eae8faee1..b42f04acf06e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -37,6 +37,7 @@
 #include <linux/uuid.h>
 #include <linux/errseq.h>
 #include <linux/ioprio.h>
+#include <linux/file_type.h>
 
 #include <asm/byteorder.h>
 #include <uapi/linux/fs.h>
@@ -1663,22 +1664,6 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
 			    u64 phys, u64 len, u32 flags);
 int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
 
-/*
- * File types
- *
- * NOTE! These match bits 12..15 of stat.st_mode
- * (ie "(i_mode >> 12) & 15").
- */
-#define DT_UNKNOWN	0
-#define DT_FIFO		1
-#define DT_CHR		2
-#define DT_DIR		4
-#define DT_BLK		6
-#define DT_REG		8
-#define DT_LNK		10
-#define DT_SOCK		12
-#define DT_WHT		14
-
 /*
  * This is the "filldir" function type, used by readdir() to let
  * the kernel specify what kind of dirent layout it wants to have.
-- 
2.17.2

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Amir Goldstein]

On Tue, Oct 23, 2018 at 11:19 PM Phillip Potter <phil@philpotter.co.uk> wrote:
>
> Many file systems use a copy&paste implementation
> of dirent to on-disk file type conversions.
>
> Create a common implementation to be used by file systems
> with some useful conversion helpers to reduce open coded
> file type conversions in file system code.
>
> Original patch written by Amir Goldstein.

Looks good.
I guess you used 'git apply' or just 'patch'
What you usually do when applying someone else mostly unchanged
patches is use 'git am -s -3' so you preserve the original author and
original commit message including the Signed-of-by line.
You can edit your patch by hand to change the From: line to change the
author and add
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
(you sign below me as you changed the patch last)

>
> v3:
> - Rebased against Linux 4.19 by Phillip Potter
> - Added SPDX tag to new include/linux/file_type.h
> - Added include/linux/file_type.h to MAINTAINERS
>

As you can see in my original posting, this part comes after the ---
line which means it will not be included in the commit message,
because the specific development history of this patch may be interesting
for reviewers but it is less interesting in the context of git log.

> v2:
> - s/DT_MASK/S_DT_MASK to fix redefinition in drivers/scsi/qla2xxx/qla_def.h
> - Explicit initialization of fs_dtype_by_ftype[] using [FT_*] = DT_*
>
> v1:
> - Initial implementation
>
> Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
> ---
>  MAINTAINERS               |   1 +
>  include/linux/file_type.h | 108 ++++++++++++++++++++++++++++++++++++++
>  include/linux/fs.h        |  17 +-----
>  3 files changed, 110 insertions(+), 16 deletions(-)
>  create mode 100644 include/linux/file_type.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b2f710eee67a..8e5b029886e6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5689,6 +5689,7 @@ L:        linux-fsdevel@vger.kernel.org
>  S:     Maintained
>  F:     fs/*
>  F:     include/linux/fs.h
> +F:     include/linux/file_type.h
>  F:     include/uapi/linux/fs.h
>
>  FINTEK F75375S HARDWARE MONITOR AND FAN CONTROLLER DRIVER
> diff --git a/include/linux/file_type.h b/include/linux/file_type.h
> new file mode 100644
> index 000000000000..f015c41ca90c
> --- /dev/null
> +++ b/include/linux/file_type.h
> @@ -0,0 +1,108 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_FILE_TYPE_H
> +#define _LINUX_FILE_TYPE_H
> +
> +/*
> + * This is a common implementation of dirent to fs on-disk
> + * file type conversion.  Although the fs on-disk bits are
> + * specific to every file system, in practice, many file systems
> + * use the exact same on-disk format to describe the lower 3
> + * file type bits that represent the 7 POSIX file types.
> + * All those file systems can use this generic code for the
> + * conversions:
> + *  i_mode -> fs on-disk file type (ftype)
> + *  fs on-disk file type (ftype) -> dirent file type (dtype)
> + *  i_mode -> dirent file type (dtype)
> + */
> +
> +/*
> + * struct dirent file types
> + * exposed to user via getdents(2), readdir(3)
> + *
> + * These match bits 12..15 of stat.st_mode
> + * (ie "(i_mode >> 12) & 15").
> + */
> +#define S_DT_SHIFT     12
> +#define S_DT(mode)     (((mode) & S_IFMT) >> S_DT_SHIFT)
> +#define S_DT_MASK      (S_IFMT >> S_DT_SHIFT)
> +
> +#define DT_UNKNOWN     0
> +#define DT_FIFO                S_DT(S_IFIFO) /* 1 */
> +#define DT_CHR         S_DT(S_IFCHR) /* 2 */
> +#define DT_DIR         S_DT(S_IFDIR) /* 4 */
> +#define DT_BLK         S_DT(S_IFBLK) /* 6 */
> +#define DT_REG         S_DT(S_IFREG) /* 8 */
> +#define DT_LNK         S_DT(S_IFLNK) /* 10 */
> +#define DT_SOCK                S_DT(S_IFSOCK) /* 12 */
> +#define DT_WHT         14
> +
> +#define DT_MAX         (S_DT_MASK + 1) /* 16 */
> +
> +/*
> + * fs on-disk file types.
> + * Only the low 3 bits are used for the POSIX file types.
> + * Other bits are reserved for fs private use.
> + *
> + * Note that no fs currently stores the whiteout type on-disk,
> + * so whiteout dirents are exposed to user as DT_CHR.
> + */
> +#define FT_UNKNOWN     0
> +#define FT_REG_FILE    1
> +#define FT_DIR         2
> +#define FT_CHRDEV      3
> +#define FT_BLKDEV      4
> +#define FT_FIFO                5
> +#define FT_SOCK                6
> +#define FT_SYMLINK     7
> +
> +#define FT_MAX         8
> +
> +/*
> + * fs on-disk file type to dirent file type conversion
> + */
> +static unsigned char fs_dtype_by_ftype[FT_MAX] = {
> +       [FT_UNKNOWN]    = DT_UNKNOWN,
> +       [FT_REG_FILE]   = DT_REG,
> +       [FT_DIR]        = DT_DIR,
> +       [FT_CHRDEV]     = DT_CHR,
> +       [FT_BLKDEV]     = DT_BLK,
> +       [FT_FIFO]       = DT_FIFO,
> +       [FT_SOCK]       = DT_SOCK,
> +       [FT_SYMLINK]    = DT_LNK
> +};
> +
> +static inline unsigned char fs_dtype(int filetype)
> +{
> +       if (filetype >= FT_MAX)
> +               return DT_UNKNOWN;
> +
> +       return fs_dtype_by_ftype[filetype];
> +}
> +
> +/*
> + * dirent file type to fs on-disk file type conversion
> + * Values not initialized explicitly are FT_UNKNOWN (0).
> + */
> +static unsigned char fs_ftype_by_dtype[DT_MAX] = {
> +       [DT_REG]        = FT_REG_FILE,
> +       [DT_DIR]        = FT_DIR,
> +       [DT_LNK]        = FT_SYMLINK,
> +       [DT_CHR]        = FT_CHRDEV,
> +       [DT_BLK]        = FT_BLKDEV,
> +       [DT_FIFO]       = FT_FIFO,
> +       [DT_SOCK]       = FT_SOCK,
> +};
> +
> +/* st_mode to fs on-disk file type conversion */
> +static inline unsigned char fs_umode_to_ftype(umode_t mode)
> +{
> +       return fs_ftype_by_dtype[S_DT(mode)];
> +}
> +
> +/* st_mode to dirent file type conversion */
> +static inline unsigned char fs_umode_to_dtype(umode_t mode)
> +{
> +       return fs_dtype(fs_umode_to_ftype(mode));
> +}
> +
> +#endif
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 897eae8faee1..b42f04acf06e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -37,6 +37,7 @@
>  #include <linux/uuid.h>
>  #include <linux/errseq.h>
>  #include <linux/ioprio.h>
> +#include <linux/file_type.h>
>
>  #include <asm/byteorder.h>
>  #include <uapi/linux/fs.h>
> @@ -1663,22 +1664,6 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
>                             u64 phys, u64 len, u32 flags);
>  int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
>
> -/*
> - * File types
> - *
> - * NOTE! These match bits 12..15 of stat.st_mode
> - * (ie "(i_mode >> 12) & 15").
> - */
> -#define DT_UNKNOWN     0
> -#define DT_FIFO                1
> -#define DT_CHR         2
> -#define DT_DIR         4
> -#define DT_BLK         6
> -#define DT_REG         8
> -#define DT_LNK         10
> -#define DT_SOCK                12
> -#define DT_WHT         14
> -
>  /*
>   * This is the "filldir" function type, used by readdir() to let
>   * the kernel specify what kind of dirent layout it wants to have.
> --
> 2.17.2
>

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Wed, Oct 24, 2018 at 09:16:20AM +0300, Amir Goldstein wrote:
> On Tue, Oct 23, 2018 at 11:19 PM Phillip Potter <phil@philpotter.co.uk> wrote:
> >
> > Many file systems use a copy&paste implementation
> > of dirent to on-disk file type conversions.
> >
> > Create a common implementation to be used by file systems
> > with some useful conversion helpers to reduce open coded
> > file type conversions in file system code.
> >
> > Original patch written by Amir Goldstein.
> 
> Looks good.
> I guess you used 'git apply' or just 'patch'
> What you usually do when applying someone else mostly unchanged
> patches is use 'git am -s -3' so you preserve the original author and
> original commit message including the Signed-of-by line.
> You can edit your patch by hand to change the From: line to change the
> author and add
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> (you sign below me as you changed the patch last)
> 

Dear Amir,

Yes, I applied each patch manually to my tree, fixed it up where needed,
then after rebuilding and testing each one I committed it and regenerated
each patch. Thank you very much for your advice, I will take it into
account and make the necessary changes. In the meantime, do I add other
tags in the order they are received also (such as Reviewed-by:) and am
I safe to add these in when I re-send the patches with the changes you
and others have suggested (or would that offend people that have
offered the tags)?

Regards,
Phil

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Amir Goldstein]

On Wed, Oct 24, 2018 at 11:21 AM Phillip Potter <phil@philpotter.co.uk> wrote:
>
> On Wed, Oct 24, 2018 at 09:16:20AM +0300, Amir Goldstein wrote:
> > On Tue, Oct 23, 2018 at 11:19 PM Phillip Potter <phil@philpotter.co.uk> wrote:
> > >
> > > Many file systems use a copy&paste implementation
> > > of dirent to on-disk file type conversions.
> > >
> > > Create a common implementation to be used by file systems
> > > with some useful conversion helpers to reduce open coded
> > > file type conversions in file system code.
> > >
> > > Original patch written by Amir Goldstein.
> >
> > Looks good.
> > I guess you used 'git apply' or just 'patch'
> > What you usually do when applying someone else mostly unchanged
> > patches is use 'git am -s -3' so you preserve the original author and
> > original commit message including the Signed-of-by line.
> > You can edit your patch by hand to change the From: line to change the
> > author and add
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > (you sign below me as you changed the patch last)
> >
>
> Dear Amir,
>
> Yes, I applied each patch manually to my tree, fixed it up where needed,
> then after rebuilding and testing each one I committed it and regenerated
> each patch. Thank you very much for your advice, I will take it into
> account and make the necessary changes. In the meantime, do I add other
> tags in the order they are received also (such as Reviewed-by:) and am
> I safe to add these in when I re-send the patches with the changes you
> and others have suggested (or would that offend people that have
> offered the tags)?
>

Reviewed-by before of after Signed-off-by.
I prefer Signed-off-by last which conceptually covers the entire patch,
the commit message including all the review tags that you may have added.

Some developers add Reviewed-by after Signed-off-by signifying the
order that things happened, so choose your own preference.

As a reviewer, and I speak only for myself, if I offered my Reviewed-by
I expect it to be removed if a future revision of the patch has changed
so I have an indication of patches that I need to re-review.
But if the patch changed very lightly, like small edits to commit message
and code nits in general, that would not invalidate my review.
When in doubt, you can always explicitly ask the reviewer.

Thanks,
Amir.

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Wed, Oct 24, 2018 at 12:20:14PM +0300, Amir Goldstein wrote:
> On Wed, Oct 24, 2018 at 11:21 AM Phillip Potter <phil@philpotter.co.uk> wrote:
> > Dear Amir,
> >
> > Yes, I applied each patch manually to my tree, fixed it up where needed,
> > then after rebuilding and testing each one I committed it and regenerated
> > each patch. Thank you very much for your advice, I will take it into
> > account and make the necessary changes. In the meantime, do I add other
> > tags in the order they are received also (such as Reviewed-by:) and am
> > I safe to add these in when I re-send the patches with the changes you
> > and others have suggested (or would that offend people that have
> > offered the tags)?
> >
> 
> Reviewed-by before of after Signed-off-by.
> I prefer Signed-off-by last which conceptually covers the entire patch,
> the commit message including all the review tags that you may have added.
> 
> Some developers add Reviewed-by after Signed-off-by signifying the
> order that things happened, so choose your own preference.
> 
> As a reviewer, and I speak only for myself, if I offered my Reviewed-by
> I expect it to be removed if a future revision of the patch has changed
> so I have an indication of patches that I need to re-review.
> But if the patch changed very lightly, like small edits to commit message
> and code nits in general, that would not invalidate my review.
> When in doubt, you can always explicitly ask the reviewer.
> 
> Thanks,
> Amir.

Dear Amir,

Thanks - I am just going to fix up the commit messages as you suggested
using git am etc. The content of the patches themselves will not change
(until further feedback is received).

Regards,
Phil

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Amir Goldstein]

On Wed, Oct 24, 2018 at 12:31 PM Phillip Potter <phil@philpotter.co.uk> wrote:
>
> On Wed, Oct 24, 2018 at 12:20:14PM +0300, Amir Goldstein wrote:
> > On Wed, Oct 24, 2018 at 11:21 AM Phillip Potter <phil@philpotter.co.uk> wrote:
> > > Dear Amir,
> > >
> > > Yes, I applied each patch manually to my tree, fixed it up where needed,
> > > then after rebuilding and testing each one I committed it and regenerated
> > > each patch. Thank you very much for your advice, I will take it into
> > > account and make the necessary changes. In the meantime, do I add other
> > > tags in the order they are received also (such as Reviewed-by:) and am
> > > I safe to add these in when I re-send the patches with the changes you
> > > and others have suggested (or would that offend people that have
> > > offered the tags)?
> > >
> >
> > Reviewed-by before of after Signed-off-by.
> > I prefer Signed-off-by last which conceptually covers the entire patch,
> > the commit message including all the review tags that you may have added.
> >
> > Some developers add Reviewed-by after Signed-off-by signifying the
> > order that things happened, so choose your own preference.
> >
> > As a reviewer, and I speak only for myself, if I offered my Reviewed-by
> > I expect it to be removed if a future revision of the patch has changed
> > so I have an indication of patches that I need to re-review.
> > But if the patch changed very lightly, like small edits to commit message
> > and code nits in general, that would not invalidate my review.
> > When in doubt, you can always explicitly ask the reviewer.
> >
> > Thanks,
> > Amir.
>
> Dear Amir,
>
> Thanks - I am just going to fix up the commit messages as you suggested
> using git am etc. The content of the patches themselves will not change
> (until further feedback is received).
>

Well, I did request to change some content (the location and the comment
above BUILD_BUG_ON section) which is relevant for several patches.
However, so far affected patched did not get any Reviewed-by.

Thanks,
Amir.

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Wed, Oct 24, 2018 at 12:44:50PM +0300, Amir Goldstein wrote:
> On Wed, Oct 24, 2018 at 12:31 PM Phillip Potter <phil@philpotter.co.uk> wrote:
> >
> > On Wed, Oct 24, 2018 at 12:20:14PM +0300, Amir Goldstein wrote:
> > > On Wed, Oct 24, 2018 at 11:21 AM Phillip Potter <phil@philpotter.co.uk> wrote:
> > > > Dear Amir,
> > > >
> > > > Yes, I applied each patch manually to my tree, fixed it up where needed,
> > > > then after rebuilding and testing each one I committed it and regenerated
> > > > each patch. Thank you very much for your advice, I will take it into
> > > > account and make the necessary changes. In the meantime, do I add other
> > > > tags in the order they are received also (such as Reviewed-by:) and am
> > > > I safe to add these in when I re-send the patches with the changes you
> > > > and others have suggested (or would that offend people that have
> > > > offered the tags)?
> > > >
> > >
> > > Reviewed-by before of after Signed-off-by.
> > > I prefer Signed-off-by last which conceptually covers the entire patch,
> > > the commit message including all the review tags that you may have added.
> > >
> > > Some developers add Reviewed-by after Signed-off-by signifying the
> > > order that things happened, so choose your own preference.
> > >
> > > As a reviewer, and I speak only for myself, if I offered my Reviewed-by
> > > I expect it to be removed if a future revision of the patch has changed
> > > so I have an indication of patches that I need to re-review.
> > > But if the patch changed very lightly, like small edits to commit message
> > > and code nits in general, that would not invalidate my review.
> > > When in doubt, you can always explicitly ask the reviewer.
> > >
> > > Thanks,
> > > Amir.
> >
> > Dear Amir,
> >
> > Thanks - I am just going to fix up the commit messages as you suggested
> > using git am etc. The content of the patches themselves will not change
> > (until further feedback is received).
> >
> 
> Well, I did request to change some content (the location and the comment
> above BUILD_BUG_ON section) which is relevant for several patches.
> However, so far affected patched did not get any Reviewed-by.
> 
> Thanks,
> Amir.

Sorry, I missed that bit at the end, was too keen to click through to the
note about the alleged ext2 bug :-) I will make sure those changes are made
as well. By location do you mean the location of the v3, v2, etc. stuff and
your point about including it in the main [0/10] message rather than the
patches themselves? Again, thank you for your feedback and for being patient
with me, I really do appreciate it.

Regards,
Phil

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Amir Goldstein]

On Wed, Oct 24, 2018 at 12:56 PM Phillip Potter <phil@philpotter.co.uk> wrote:
>
> On Wed, Oct 24, 2018 at 12:44:50PM +0300, Amir Goldstein wrote:
...
> > Well, I did request to change some content (the location and the comment
> > above BUILD_BUG_ON section) which is relevant for several patches.
> > However, so far affected patched did not get any Reviewed-by.
> >
> > Thanks,
> > Amir.
>
> Sorry, I missed that bit at the end, was too keen to click through to the
> note about the alleged ext2 bug :-) I will make sure those changes are made
> as well. By location do you mean the location of the v3, v2, etc. stuff and
> your point about including it in the main [0/10] message rather than the
> patches themselves? Again, thank you for your feedback and for being patient
> with me, I really do appreciate it.
>

By location I meant place BUILD_BUG_ON() at the beginning of
ext2_set_de_type() and not nested inside inner scope of ext2_readdir().
And find similar appropriate places for other patches.

And one more thing for next posting - LKML is probably a too wide forum for
this filesystems cleanup series.

Cheers,
Amir.

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Al Viro]

On Tue, Oct 23, 2018 at 09:19:53PM +0100, Phillip Potter wrote:

> +static inline unsigned char fs_dtype(int filetype)

That "int" is asking for trouble, especially since negative
argument will blow up.  And it comes from untrusted source...

> +{
> +	if (filetype >= FT_MAX)
> +		return DT_UNKNOWN;
> +
> +	return fs_dtype_by_ftype[filetype];
> +}

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Theodore Y. Ts'o]

On Tue, Oct 23, 2018 at 09:19:53PM +0100, Phillip Potter wrote:
> diff --git a/include/linux/file_type.h b/include/linux/file_type.h

Shouldn't this be in include/uapi/linux/fs_types.h?

One of things which must be made crystal clear is these definitions
MUST NOT ever change.  It would break the Userspace ABI, and would
break file systems on-disk format.

It might also be useful to be clear *why* we are making this change in
the first place.  Code refactorization is good from a code maintenance
perspective (either to fix bugs, although this code is pretty
trivial), or to make it easier to make changes in a single central
place (which MUST NOT) happen, or to make the compiled code more
compact.

So some documentation of how much text is actually saved might be
worthwhile.

						- Ted

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Wed, Oct 24, 2018 at 01:37:40PM +0100, Al Viro wrote:
> On Tue, Oct 23, 2018 at 09:19:53PM +0100, Phillip Potter wrote:
> 
> > +static inline unsigned char fs_dtype(int filetype)
> 
> That "int" is asking for trouble, especially since negative
> argument will blow up.  And it comes from untrusted source...
> 
> > +{
> > +	if (filetype >= FT_MAX)
> > +		return DT_UNKNOWN;
> > +
> > +	return fs_dtype_by_ftype[filetype];
> > +}

Dear Al,

Thank you, good point, I will change to unsigned int and
republish as part of new series.

Regards,
Phil

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Wed, Oct 24, 2018 at 09:02:06AM -0400, Theodore Y. Ts'o wrote:
> On Tue, Oct 23, 2018 at 09:19:53PM +0100, Phillip Potter wrote:
> > diff --git a/include/linux/file_type.h b/include/linux/file_type.h
> 
> Shouldn't this be in include/uapi/linux/fs_types.h?
> 
> One of things which must be made crystal clear is these definitions
> MUST NOT ever change.  It would break the Userspace ABI, and would
> break file systems on-disk format.
> 
> It might also be useful to be clear *why* we are making this change in
> the first place.  Code refactorization is good from a code maintenance
> perspective (either to fix bugs, although this code is pretty
> trivial), or to make it easier to make changes in a single central
> place (which MUST NOT) happen, or to make the compiled code more
> compact.
> 
> So some documentation of how much text is actually saved might be
> worthwhile.
> 
> 						- Ted

Dear Ted,

Regarding location of the additional header, thank you for this
suggestion - I will move it.

As for making it extra clear that the definitions must not change, I
will add this to the comment at the beginning of the file, and also
in the intro message of the new series I am about to publish.

I can't speak for Amir, but regarding why I think this would be a
good change, my thought process is that it would make adding new
file systems that use these on-disk type layouts in future more
easy, as the central generic implementation can be used rather than
copying and pasting and changing names etc. as needed, which is
more error prone. I am happy to add more documentation in this regard,
and also mention savings where appropriate.

Regards,
Phil

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Amir Goldstein]

On Wed, Oct 24, 2018 at 4:02 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Tue, Oct 23, 2018 at 09:19:53PM +0100, Phillip Potter wrote:
> > diff --git a/include/linux/file_type.h b/include/linux/file_type.h
>
> Shouldn't this be in include/uapi/linux/fs_types.h?
>

IDGI. Why do we want this file in uapi?
The DT_ constants are already defined by glibc dirent.h
and the FT_ constants and macros we don't want to expose
to uapi at all. Right?

Maybe all we need is a comment above DT_ constants
that those are defined by POSIX and in glibc dirent.h?

> One of things which must be made crystal clear is these definitions
> MUST NOT ever change.  It would break the Userspace ABI, and would
> break file systems on-disk format.
>
> It might also be useful to be clear *why* we are making this change in
> the first place.  Code refactorization is good from a code maintenance
> perspective (either to fix bugs, although this code is pretty
> trivial),

Very trivial code that has had an out of bounds access bug for two
decades and bug was duplicated to 7 filesystems. IMO, fixing the bug in
one place instead of 7 is a good enough reason for re-factoring.

Thanks,
Amir.

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Wed, Oct 24, 2018 at 05:41:15PM +0300, Amir Goldstein wrote:
> On Wed, Oct 24, 2018 at 4:02 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
> >
> > On Tue, Oct 23, 2018 at 09:19:53PM +0100, Phillip Potter wrote:
> > > diff --git a/include/linux/file_type.h b/include/linux/file_type.h
> >
> > Shouldn't this be in include/uapi/linux/fs_types.h?
> >
> 
> IDGI. Why do we want this file in uapi?
> The DT_ constants are already defined by glibc dirent.h
> and the FT_ constants and macros we don't want to expose
> to uapi at all. Right?
> 
> Maybe all we need is a comment above DT_ constants
> that those are defined by POSIX and in glibc dirent.h?
> 
> > One of things which must be made crystal clear is these definitions
> > MUST NOT ever change.  It would break the Userspace ABI, and would
> > break file systems on-disk format.
> >
> > It might also be useful to be clear *why* we are making this change in
> > the first place.  Code refactorization is good from a code maintenance
> > perspective (either to fix bugs, although this code is pretty
> > trivial),
> 
> Very trivial code that has had an out of bounds access bug for two
> decades and bug was duplicated to 7 filesystems. IMO, fixing the bug in
> one place instead of 7 is a good enough reason for re-factoring.
> 
> Thanks,
> Amir.

Dear Amir,

Regarding the location of the extra header file, I'm happy to stick it
in either include/linux or include/uapi/linux before sending out the
revised series. You make a valid point about the FT_ constants and macros
being kernel only though, so perhaps the comment is a better option? I
will wait a bit before sending out as I'm interested to see what Ted thinks.
Many thanks.

Regards,
Phil

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Jan Kara]

On Tue 23-10-18 21:19:53, Phillip Potter wrote:
> Many file systems use a copy&paste implementation
> of dirent to on-disk file type conversions.
> 
> Create a common implementation to be used by file systems
> with some useful conversion helpers to reduce open coded
> file type conversions in file system code.

Thanks for the patch. I like the cleanup. Some comments inline...

> diff --git a/include/linux/file_type.h b/include/linux/file_type.h
> new file mode 100644
> index 000000000000..f015c41ca90c
> --- /dev/null
> +++ b/include/linux/file_type.h
> @@ -0,0 +1,108 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_FILE_TYPE_H
> +#define _LINUX_FILE_TYPE_H
> +
> +/*
> + * This is a common implementation of dirent to fs on-disk
> + * file type conversion.  Although the fs on-disk bits are
> + * specific to every file system, in practice, many file systems
> + * use the exact same on-disk format to describe the lower 3
> + * file type bits that represent the 7 POSIX file types.
> + * All those file systems can use this generic code for the
> + * conversions:
> + *  i_mode -> fs on-disk file type (ftype)
> + *  fs on-disk file type (ftype) -> dirent file type (dtype)
> + *  i_mode -> dirent file type (dtype)

I don't think these three lines above are really useful. If you want to
make it easier for fs developers, add Docbook coments at function
implementations.

> + */
> +
> +/*
> + * struct dirent file types
> + * exposed to user via getdents(2), readdir(3)
> + *
> + * These match bits 12..15 of stat.st_mode
> + * (ie "(i_mode >> 12) & 15").
> + */
> +#define S_DT_SHIFT	12
> +#define S_DT(mode)	(((mode) & S_IFMT) >> S_DT_SHIFT)
> +#define S_DT_MASK	(S_IFMT >> S_DT_SHIFT)
> +
> +#define DT_UNKNOWN	0
> +#define DT_FIFO		S_DT(S_IFIFO) /* 1 */
> +#define DT_CHR		S_DT(S_IFCHR) /* 2 */
> +#define DT_DIR		S_DT(S_IFDIR) /* 4 */
> +#define DT_BLK		S_DT(S_IFBLK) /* 6 */
> +#define DT_REG		S_DT(S_IFREG) /* 8 */
> +#define DT_LNK		S_DT(S_IFLNK) /* 10 */
> +#define DT_SOCK		S_DT(S_IFSOCK) /* 12 */

Why not keep the original definition by absolute number. After all these
must match glibc...

> +#define DT_WHT		14
> +
> +#define DT_MAX		(S_DT_MASK + 1) /* 16 */
> +
> +/*
> + * fs on-disk file types.
> + * Only the low 3 bits are used for the POSIX file types.
> + * Other bits are reserved for fs private use.
> + *
> + * Note that no fs currently stores the whiteout type on-disk,
> + * so whiteout dirents are exposed to user as DT_CHR.
> + */
> +#define FT_UNKNOWN	0
> +#define FT_REG_FILE	1
> +#define FT_DIR		2
> +#define FT_CHRDEV	3
> +#define FT_BLKDEV	4
> +#define FT_FIFO		5
> +#define FT_SOCK		6
> +#define FT_SYMLINK	7
> +
> +#define FT_MAX		8
> +
> +/*
> + * fs on-disk file type to dirent file type conversion
> + */
> +static unsigned char fs_dtype_by_ftype[FT_MAX] = {
> +	[FT_UNKNOWN]	= DT_UNKNOWN,
> +	[FT_REG_FILE]	= DT_REG,
> +	[FT_DIR]	= DT_DIR,
> +	[FT_CHRDEV]	= DT_CHR,
> +	[FT_BLKDEV]	= DT_BLK,
> +	[FT_FIFO]	= DT_FIFO,
> +	[FT_SOCK]	= DT_SOCK,
> +	[FT_SYMLINK]	= DT_LNK
> +};

So you define static variable in a header file. That will make it appear in
each and every object that includes this header (when not used it will get
optimized away but still...). IMO that is not good and I don't think
anything here is so performance super-crittical, that the cost of
additional function call would matter (correct me if I'm wrong here). So
I'd rather see these tables and associated functions in some C file.

> +static inline unsigned char fs_dtype(int filetype)

This function name is not very descriptive and consistent with the other
two. It should be like fs_ftype_to_dtype(), right?

> +{
> +	if (filetype >= FT_MAX)
> +		return DT_UNKNOWN;
> +
> +	return fs_dtype_by_ftype[filetype];
> +}

Otherwise the patch looks good to me.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Thu, Oct 25, 2018 at 01:20:44PM +0200, Jan Kara wrote:
> On Tue 23-10-18 21:19:53, Phillip Potter wrote:
> > Many file systems use a copy&paste implementation
> > of dirent to on-disk file type conversions.
> > 
> > Create a common implementation to be used by file systems
> > with some useful conversion helpers to reduce open coded
> > file type conversions in file system code.
> 
> Thanks for the patch. I like the cleanup. Some comments inline...
> 
> > diff --git a/include/linux/file_type.h b/include/linux/file_type.h
> > new file mode 100644
> > index 000000000000..f015c41ca90c
> > --- /dev/null
> > +++ b/include/linux/file_type.h
> > @@ -0,0 +1,108 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _LINUX_FILE_TYPE_H
> > +#define _LINUX_FILE_TYPE_H
> > +
> > +/*
> > + * This is a common implementation of dirent to fs on-disk
> > + * file type conversion.  Although the fs on-disk bits are
> > + * specific to every file system, in practice, many file systems
> > + * use the exact same on-disk format to describe the lower 3
> > + * file type bits that represent the 7 POSIX file types.
> > + * All those file systems can use this generic code for the
> > + * conversions:
> > + *  i_mode -> fs on-disk file type (ftype)
> > + *  fs on-disk file type (ftype) -> dirent file type (dtype)
> > + *  i_mode -> dirent file type (dtype)
> 
> I don't think these three lines above are really useful. If you want to
> make it easier for fs developers, add Docbook coments at function
> implementations.
> 
> > + */
> > +
> > +/*
> > + * struct dirent file types
> > + * exposed to user via getdents(2), readdir(3)
> > + *
> > + * These match bits 12..15 of stat.st_mode
> > + * (ie "(i_mode >> 12) & 15").
> > + */
> > +#define S_DT_SHIFT	12
> > +#define S_DT(mode)	(((mode) & S_IFMT) >> S_DT_SHIFT)
> > +#define S_DT_MASK	(S_IFMT >> S_DT_SHIFT)
> > +
> > +#define DT_UNKNOWN	0
> > +#define DT_FIFO		S_DT(S_IFIFO) /* 1 */
> > +#define DT_CHR		S_DT(S_IFCHR) /* 2 */
> > +#define DT_DIR		S_DT(S_IFDIR) /* 4 */
> > +#define DT_BLK		S_DT(S_IFBLK) /* 6 */
> > +#define DT_REG		S_DT(S_IFREG) /* 8 */
> > +#define DT_LNK		S_DT(S_IFLNK) /* 10 */
> > +#define DT_SOCK		S_DT(S_IFSOCK) /* 12 */
> 
> Why not keep the original definition by absolute number. After all these
> must match glibc...
> 
> > +#define DT_WHT		14
> > +
> > +#define DT_MAX		(S_DT_MASK + 1) /* 16 */
> > +
> > +/*
> > + * fs on-disk file types.
> > + * Only the low 3 bits are used for the POSIX file types.
> > + * Other bits are reserved for fs private use.
> > + *
> > + * Note that no fs currently stores the whiteout type on-disk,
> > + * so whiteout dirents are exposed to user as DT_CHR.
> > + */
> > +#define FT_UNKNOWN	0
> > +#define FT_REG_FILE	1
> > +#define FT_DIR		2
> > +#define FT_CHRDEV	3
> > +#define FT_BLKDEV	4
> > +#define FT_FIFO		5
> > +#define FT_SOCK		6
> > +#define FT_SYMLINK	7
> > +
> > +#define FT_MAX		8
> > +
> > +/*
> > + * fs on-disk file type to dirent file type conversion
> > + */
> > +static unsigned char fs_dtype_by_ftype[FT_MAX] = {
> > +	[FT_UNKNOWN]	= DT_UNKNOWN,
> > +	[FT_REG_FILE]	= DT_REG,
> > +	[FT_DIR]	= DT_DIR,
> > +	[FT_CHRDEV]	= DT_CHR,
> > +	[FT_BLKDEV]	= DT_BLK,
> > +	[FT_FIFO]	= DT_FIFO,
> > +	[FT_SOCK]	= DT_SOCK,
> > +	[FT_SYMLINK]	= DT_LNK
> > +};
> 
> So you define static variable in a header file. That will make it appear in
> each and every object that includes this header (when not used it will get
> optimized away but still...). IMO that is not good and I don't think
> anything here is so performance super-crittical, that the cost of
> additional function call would matter (correct me if I'm wrong here). So
> I'd rather see these tables and associated functions in some C file.
> 
> > +static inline unsigned char fs_dtype(int filetype)
> 
> This function name is not very descriptive and consistent with the other
> two. It should be like fs_ftype_to_dtype(), right?
> 
> > +{
> > +	if (filetype >= FT_MAX)
> > +		return DT_UNKNOWN;
> > +
> > +	return fs_dtype_by_ftype[filetype];
> > +}
> 
> Otherwise the patch looks good to me.
> 
> 								Honza
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Dear Jan,

Thanks for your comments/feedback, really appreciate it. All good points
and I will make sure I act on it all before republishing the series. I'd
be interested to know your thoughts on Ted T'so's suggestion about moving
the new header to the uapi area - yes or no in your opinion? Happy with
either, but looking for the widest possible acceptance. Thanks.

Regards,
Phil

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Jan Kara]

On Thu 25-10-18 13:56:23, Phillip Potter wrote:
> Thanks for your comments/feedback, really appreciate it. All good points
> and I will make sure I act on it all before republishing the series. I'd
> be interested to know your thoughts on Ted T'so's suggestion about moving
> the new header to the uapi area - yes or no in your opinion? Happy with
> either, but looking for the widest possible acceptance. Thanks.

I don't think moving this to uapi headers makes sense. As Amir wrote, all
necessary definitions for userspace already come from glibc headers.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Thu, Oct 25, 2018 at 03:42:59PM +0200, Jan Kara wrote:
> On Thu 25-10-18 13:56:23, Phillip Potter wrote:
> > Thanks for your comments/feedback, really appreciate it. All good points
> > and I will make sure I act on it all before republishing the series. I'd
> > be interested to know your thoughts on Ted T'so's suggestion about moving
> > the new header to the uapi area - yes or no in your opinion? Happy with
> > either, but looking for the widest possible acceptance. Thanks.
> 
> I don't think moving this to uapi headers makes sense. As Amir wrote, all
> necessary definitions for userspace already come from glibc headers.
> 
> 								Honza
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Thanks, I will leave it in include/linux for now then and separate out the
relevant parts to a separate C file as you suggested.

Regards,
Phil

Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

On Wed, Oct 24, 2018 at 09:16:20AM +0300, Amir Goldstein wrote:
> On Tue, Oct 23, 2018 at 11:19 PM Phillip Potter <phil@philpotter.co.uk> wrote:
> >
> > Many file systems use a copy&paste implementation
> > of dirent to on-disk file type conversions.
> >
> > Create a common implementation to be used by file systems
> > with some useful conversion helpers to reduce open coded
> > file type conversions in file system code.
> >
> > Original patch written by Amir Goldstein.
> 
> Looks good.
> I guess you used 'git apply' or just 'patch'
> What you usually do when applying someone else mostly unchanged
> patches is use 'git am -s -3' so you preserve the original author and
> original commit message including the Signed-of-by line.
> You can edit your patch by hand to change the From: line to change the
> author and add
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> (you sign below me as you changed the patch last)
>

Dear Amir,

I am almost ready to send out the new series with all the suggestions so far
incorporated. Would you be happy for me to add to the commit messages slightly
to mention David Sterba's point about a brief explanation? It would be the same
in each case and I would keep it brief. Also, I have split the functions into a
C file as per Jan Kara's suggestion. Are you still happy for me to still include
your Signed-off-by tag or would you rather I keep it out until you've had a
look? Thanks.

Regards,
Phil

[RFC][PATCH v3 01/10] fs: common implementation of file type

[Phillip Potter]

Many file systems use a copy&paste implementation
of dirent to on-disk file type conversions.

Create a common implementation to be used by file systems
with some useful conversion helpers to reduce open coded
file type conversions in file system code.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
---
 MAINTAINERS              |   1 +
 fs/Makefile              |   3 +-
 fs/fs_types.c            | 105 +++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h       |  17 +------
 include/linux/fs_types.h |  73 +++++++++++++++++++++++++++
 5 files changed, 182 insertions(+), 17 deletions(-)
 create mode 100644 fs/fs_types.c
 create mode 100644 include/linux/fs_types.h

diff --git a/MAINTAINERS b/MAINTAINERS
index bd702ad56c7f..9491208c115f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5711,6 +5711,7 @@ L:	linux-fsdevel@vger.kernel.org
 S:	Maintained
 F:	fs/*
 F:	include/linux/fs.h
+F:	include/linux/fs_types.h
 F:	include/uapi/linux/fs.h
 
 FINTEK F75375S HARDWARE MONITOR AND FAN CONTROLLER DRIVER
diff --git a/fs/Makefile b/fs/Makefile
index 293733f61594..23fcd8c164a3 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -12,7 +12,8 @@ obj-y :=	open.o read_write.o file_table.o super.o \
 		attr.o bad_inode.o file.o filesystems.o namespace.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o splice.o sync.o utimes.o d_path.o \
-		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o
+		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o \
+		fs_types.o
 
 ifeq ($(CONFIG_BLOCK),y)
 obj-y +=	buffer.o block_dev.o direct-io.o mpage.o
diff --git a/fs/fs_types.c b/fs/fs_types.c
new file mode 100644
index 000000000000..6fc57f4b1dcb
--- /dev/null
+++ b/fs/fs_types.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/fs.h>
+#include <linux/export.h>
+
+/*
+ * fs on-disk file type to dirent file type conversion
+ */
+static unsigned char fs_dtype_by_ftype[FT_MAX] = {
+	[FT_UNKNOWN]	= DT_UNKNOWN,
+	[FT_REG_FILE]	= DT_REG,
+	[FT_DIR]	= DT_DIR,
+	[FT_CHRDEV]	= DT_CHR,
+	[FT_BLKDEV]	= DT_BLK,
+	[FT_FIFO]	= DT_FIFO,
+	[FT_SOCK]	= DT_SOCK,
+	[FT_SYMLINK]	= DT_LNK
+};
+
+/**
+ * fs_ftype_to_dtype() - fs on-disk file type to dirent type.
+ * @filetype: The on-disk file type to convert.
+ *
+ * This function converts the on-disk file type value (FT_*) to the directory
+ * entry type (DT_*).
+ *
+ * Context: Any context.
+ * Return:
+ * * DT_UNKNOWN		- Unknown type
+ * * DT_FIFO		- FIFO
+ * * DT_CHR		- Character device
+ * * DT_DIR		- Directory
+ * * DT_BLK		- Block device
+ * * DT_REG		- Regular file
+ * * DT_LNK		- Symbolic link
+ * * DT_SOCK		- Local-domain socket
+ */
+unsigned char fs_ftype_to_dtype(unsigned int filetype)
+{
+	if (filetype >= FT_MAX)
+		return DT_UNKNOWN;
+
+	return fs_dtype_by_ftype[filetype];
+}
+EXPORT_SYMBOL_GPL(fs_ftype_to_dtype);
+
+/*
+ * dirent file type to fs on-disk file type conversion
+ * Values not initialized explicitly are FT_UNKNOWN (0).
+ */
+static unsigned char fs_ftype_by_dtype[DT_MAX] = {
+	[DT_REG]	= FT_REG_FILE,
+	[DT_DIR]	= FT_DIR,
+	[DT_LNK]	= FT_SYMLINK,
+	[DT_CHR]	= FT_CHRDEV,
+	[DT_BLK]	= FT_BLKDEV,
+	[DT_FIFO]	= FT_FIFO,
+	[DT_SOCK]	= FT_SOCK,
+};
+
+/**
+ * fs_umode_to_ftype() - file mode to on-disk file type.
+ * @mode: The file mode to convert.
+ *
+ * This function converts the file mode value to the on-disk file type (FT_*).
+ *
+ * Context: Any context.
+ * Return:
+ * * FT_UNKNOWN		- Unknown type
+ * * FT_REG_FILE	- Regular file
+ * * FT_DIR		- Directory
+ * * FT_CHRDEV		- Character device
+ * * FT_BLKDEV		- Block device
+ * * FT_FIFO		- FIFO
+ * * FT_SOCK		- Local-domain socket
+ * * FT_SYMLINK		- Symbolic link
+ */
+unsigned char fs_umode_to_ftype(umode_t mode)
+{
+	return fs_ftype_by_dtype[S_DT(mode)];
+}
+EXPORT_SYMBOL_GPL(fs_umode_to_ftype);
+
+/**
+ * fs_umode_to_dtype() - file mode to dirent file type.
+ * @mode: The file mode to convert.
+ *
+ * This function converts the file mode value to the directory
+ * entry type (DT_*).
+ *
+ * Context: Any context.
+ * Return:
+ * * DT_UNKNOWN		- Unknown type
+ * * DT_FIFO		- FIFO
+ * * DT_CHR		- Character device
+ * * DT_DIR		- Directory
+ * * DT_BLK		- Block device
+ * * DT_REG		- Regular file
+ * * DT_LNK		- Symbolic link
+ * * DT_SOCK		- Local-domain socket
+ */
+unsigned char fs_umode_to_dtype(umode_t mode)
+{
+	return fs_ftype_to_dtype(fs_umode_to_ftype(mode));
+}
+EXPORT_SYMBOL_GPL(fs_umode_to_dtype);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 897eae8faee1..1dc76c9ea309 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -37,6 +37,7 @@
 #include <linux/uuid.h>
 #include <linux/errseq.h>
 #include <linux/ioprio.h>
+#include <linux/fs_types.h>
 
 #include <asm/byteorder.h>
 #include <uapi/linux/fs.h>
@@ -1663,22 +1664,6 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
 			    u64 phys, u64 len, u32 flags);
 int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
 
-/*
- * File types
- *
- * NOTE! These match bits 12..15 of stat.st_mode
- * (ie "(i_mode >> 12) & 15").
- */
-#define DT_UNKNOWN	0
-#define DT_FIFO		1
-#define DT_CHR		2
-#define DT_DIR		4
-#define DT_BLK		6
-#define DT_REG		8
-#define DT_LNK		10
-#define DT_SOCK		12
-#define DT_WHT		14
-
 /*
  * This is the "filldir" function type, used by readdir() to let
  * the kernel specify what kind of dirent layout it wants to have.
diff --git a/include/linux/fs_types.h b/include/linux/fs_types.h
new file mode 100644
index 000000000000..7d147b2f1702
--- /dev/null
+++ b/include/linux/fs_types.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_FS_TYPES_H
+#define _LINUX_FS_TYPES_H
+
+/*
+ * This is a header for the common implementation of dirent
+ * to fs on-disk file type conversion.  Although the fs on-disk
+ * bits are specific to every file system, in practice, many
+ * file systems use the exact same on-disk format to describe
+ * the lower 3 file type bits that represent the 7 POSIX file
+ * types.
+ *
+ * It is important to note that the definitions in this
+ * header MUST NOT change. This would break both the
+ * userspace ABI and the on-disk format of filesystems
+ * using this code.
+ *
+ * All those file systems can use this generic code for the
+ * conversions.
+ */
+
+/*
+ * struct dirent file types
+ * exposed to user via getdents(2), readdir(3)
+ *
+ * These match bits 12..15 of stat.st_mode
+ * (ie "(i_mode >> 12) & 15").
+ */
+#define S_DT_SHIFT	12
+#define S_DT(mode)	(((mode) & S_IFMT) >> S_DT_SHIFT)
+#define S_DT_MASK	(S_IFMT >> S_DT_SHIFT)
+
+/* these are defined by POSIX and also present in glibc's dirent.h */
+#define DT_UNKNOWN	0
+#define DT_FIFO		1
+#define DT_CHR		2
+#define DT_DIR		4
+#define DT_BLK		6
+#define DT_REG		8
+#define DT_LNK		10
+#define DT_SOCK		12
+#define DT_WHT		14
+
+#define DT_MAX		(S_DT_MASK + 1) /* 16 */
+
+/*
+ * fs on-disk file types.
+ * Only the low 3 bits are used for the POSIX file types.
+ * Other bits are reserved for fs private use.
+ *
+ * Note that no fs currently stores the whiteout type on-disk,
+ * so whiteout dirents are exposed to user as DT_CHR.
+ */
+#define FT_UNKNOWN	0
+#define FT_REG_FILE	1
+#define FT_DIR		2
+#define FT_CHRDEV	3
+#define FT_BLKDEV	4
+#define FT_FIFO		5
+#define FT_SOCK		6
+#define FT_SYMLINK	7
+
+#define FT_MAX		8
+
+/*
+ * declarations for helper functions, accompanying implementation
+ * is in fs/fs_types.c
+ */
+extern unsigned char fs_ftype_to_dtype(unsigned int filetype);
+extern unsigned char fs_umode_to_ftype(umode_t mode);
+extern unsigned char fs_umode_to_dtype(umode_t mode);
+
+#endif
-- 
2.17.2