As per the original TODO, replacing the switch statement with a lookup
table results in more concise mapping logic and improved performance.
ufs_mode_to_dt maps file mode bits to directory entry types. Indices are
created by right shifting 12 bits to isolate the higher-order bits in
order to use smaller indexing values.
e.g. S_IFSOCK >> FT_SHIFT => 0140000 >> 12 = 1100
Signed-off-by: Luis Felipe Hernandez <[email protected]>
---
fs/ufs/util.h | 46 +++++++++++++++++++---------------------------
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/fs/ufs/util.h b/fs/ufs/util.h
index 0ecd2ed792f5..caa843373191 100644
--- a/fs/ufs/util.h
+++ b/fs/ufs/util.h
@@ -7,10 +7,13 @@
* Charles University, Faculty of Mathematics and Physics
*/
+#include <linux/array_size.h>
#include <linux/buffer_head.h>
#include <linux/fs.h>
#include "swab.h"
+#define FT_SHIFT 12
+
/*
* functions used for retyping
*/
@@ -146,40 +149,29 @@ ufs_set_de_namlen(struct super_block *sb, struct ufs_dir_entry *de, u16 value)
de->d_u.d_44.d_namlen = value; /* XXX this seems wrong */
}
+static const unsigned char ufs_mode_to_dt[] = {
+ [S_IFSOCK >> FT_SHIFT] = DT_SOCK,
+ [S_IFLNK >> FT_SHIFT] = DT_LNK,
+ [S_IFREG >> FT_SHIFT] = DT_REG,
+ [S_IFBLK >> FT_SHIFT] = DT_BLK,
+ [S_IFDIR >> FT_SHIFT] = DT_DIR,
+ [S_IFCHR >> FT_SHIFT] = DT_CHR,
+ [S_IFIFO >> FT_SHIFT] = DT_FIFO,
+ [0] = DT_UNKNOWN
+};
+
static inline void
ufs_set_de_type(struct super_block *sb, struct ufs_dir_entry *de, int mode)
{
if ((UFS_SB(sb)->s_flags & UFS_DE_MASK) != UFS_DE_44BSD)
return;
- /*
- * TODO turn this into a table lookup
- */
- switch (mode & S_IFMT) {
- case S_IFSOCK:
- de->d_u.d_44.d_type = DT_SOCK;
- break;
- case S_IFLNK:
- de->d_u.d_44.d_type = DT_LNK;
- break;
- case S_IFREG:
- de->d_u.d_44.d_type = DT_REG;
- break;
- case S_IFBLK:
- de->d_u.d_44.d_type = DT_BLK;
- break;
- case S_IFDIR:
- de->d_u.d_44.d_type = DT_DIR;
- break;
- case S_IFCHR:
- de->d_u.d_44.d_type = DT_CHR;
- break;
- case S_IFIFO:
- de->d_u.d_44.d_type = DT_FIFO;
- break;
- default:
+ unsigned int mode_index = (mode & S_IFMT) >> FT_SHIFT;
+
+ if (mode_index < ARRAY_SIZE(ufs_mode_to_dt))
+ de->d_u.d_44.d_type = ufs_mode_to_dt[mode_index];
+ else
de->d_u.d_44.d_type = DT_UNKNOWN;
- }
}
static inline u32
--
2.45.2
> As per the original TODO, replacing the switch statement with a lookup
> table results in more concise mapping logic and improved performance.
…
Can imperative wordings be relevant for another improved change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc2#n94
…
> +++ b/fs/ufs/util.h
…
> static inline void
> ufs_set_de_type(struct super_block *sb, struct ufs_dir_entry *de, int mode)
…
> + if (mode_index < ARRAY_SIZE(ufs_mode_to_dt))
> + de->d_u.d_44.d_type = ufs_mode_to_dt[mode_index];
> + else
> de->d_u.d_44.d_type = DT_UNKNOWN;
…
May a conditional operator expression be applied at this source code place?
Regards,
Markus
On Mon, Jun 10, 2024 at 09:48:09AM +0200, Markus Elfring wrote:
> > As per the original TODO, replacing the switch statement with a lookup
> > table results in more concise mapping logic and improved performance.
> …
>
> Can imperative wordings be relevant for another improved change description?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc2#n94
>
>
> …
> > +++ b/fs/ufs/util.h
> …
> > static inline void
> > ufs_set_de_type(struct super_block *sb, struct ufs_dir_entry *de, int mode)
> …
> > + if (mode_index < ARRAY_SIZE(ufs_mode_to_dt))
> > + de->d_u.d_44.d_type = ufs_mode_to_dt[mode_index];
> > + else
> > de->d_u.d_44.d_type = DT_UNKNOWN;
> …
>
> May a conditional operator expression be applied at this source code place?
>
> Regards,
> Markus
>
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list. I strongly suggest that you not do this anymore. Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all. The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback. Please feel free to also ignore emails
from them.
thanks,
greg k-h's patch email bot