include/linux/fs.h | 58 +++++++++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 29 deletions(-)
Christoph says, in response to one of the patches in the i_ino widening
series, which changes the prototype of several functions in fs.h:
"Can you please drop all these pointless externs while you're at it?"
Remove extern keyword from functions touched by that patch (and a few
that happened to be nearby).
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
include/linux/fs.h | 58 +++++++++++++++++++++++++++---------------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 097443bf12e289c347651e5f3da5b67eb6b53121..0c3ad6d7a20055b654b6d5087756f33d9e0fc4fc 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2912,36 +2912,36 @@ static inline bool name_contains_dotdot(const char *name)
#include <linux/err.h>
/* needed for stackable file system support */
-extern loff_t default_llseek(struct file *file, loff_t offset, int whence);
+loff_t default_llseek(struct file *file, loff_t offset, int whence);
-extern loff_t vfs_llseek(struct file *file, loff_t offset, int whence);
+loff_t vfs_llseek(struct file *file, loff_t offset, int whence);
-extern int inode_init_always_gfp(struct super_block *, struct inode *, gfp_t);
+int inode_init_always_gfp(struct super_block *, struct inode *, gfp_t);
static inline int inode_init_always(struct super_block *sb, struct inode *inode)
{
return inode_init_always_gfp(sb, inode, GFP_NOFS);
}
-extern void inode_init_once(struct inode *);
-extern void address_space_init_once(struct address_space *mapping);
-extern struct inode * igrab(struct inode *);
-extern ino_t iunique(struct super_block *, ino_t);
-extern int inode_needs_sync(struct inode *inode);
-extern int inode_just_drop(struct inode *inode);
+void inode_init_once(struct inode *);
+void address_space_init_once(struct address_space *mapping);
+struct inode *igrab(struct inode *);
+ino_t iunique(struct super_block *, ino_t);
+int inode_needs_sync(struct inode *inode);
+int inode_just_drop(struct inode *inode);
static inline int inode_generic_drop(struct inode *inode)
{
return !inode->i_nlink || inode_unhashed(inode);
}
-extern void d_mark_dontcache(struct inode *inode);
+void d_mark_dontcache(struct inode *inode);
-extern struct inode *ilookup5_nowait(struct super_block *sb,
+struct inode *ilookup5_nowait(struct super_block *sb,
u64 hashval, int (*test)(struct inode *, void *),
void *data, bool *isnew);
-extern struct inode *ilookup5(struct super_block *sb, u64 hashval,
+struct inode *ilookup5(struct super_block *sb, u64 hashval,
int (*test)(struct inode *, void *), void *data);
-extern struct inode *ilookup(struct super_block *sb, u64 ino);
+struct inode *ilookup(struct super_block *sb, u64 ino);
-extern struct inode *inode_insert5(struct inode *inode, u64 hashval,
+struct inode *inode_insert5(struct inode *inode, u64 hashval,
int (*test)(struct inode *, void *),
int (*set)(struct inode *, void *),
void *data);
@@ -2951,26 +2951,26 @@ struct inode *iget5_locked(struct super_block *, u64,
struct inode *iget5_locked_rcu(struct super_block *, u64,
int (*test)(struct inode *, void *),
int (*set)(struct inode *, void *), void *);
-extern struct inode *iget_locked(struct super_block *, u64);
-extern struct inode *find_inode_nowait(struct super_block *,
+struct inode *iget_locked(struct super_block *, u64);
+struct inode *find_inode_nowait(struct super_block *,
u64,
int (*match)(struct inode *,
u64, void *),
void *data);
-extern struct inode *find_inode_rcu(struct super_block *, u64,
+struct inode *find_inode_rcu(struct super_block *, u64,
int (*)(struct inode *, void *), void *);
-extern struct inode *find_inode_by_ino_rcu(struct super_block *, u64);
-extern int insert_inode_locked4(struct inode *, u64, int (*test)(struct inode *, void *), void *);
-extern int insert_inode_locked(struct inode *);
+struct inode *find_inode_by_ino_rcu(struct super_block *, u64);
+int insert_inode_locked4(struct inode *, u64, int (*test)(struct inode *, void *), void *);
+int insert_inode_locked(struct inode *);
#ifdef CONFIG_DEBUG_LOCK_ALLOC
-extern void lockdep_annotate_inode_mutex_key(struct inode *inode);
+void lockdep_annotate_inode_mutex_key(struct inode *inode);
#else
static inline void lockdep_annotate_inode_mutex_key(struct inode *inode) { };
#endif
-extern void unlock_new_inode(struct inode *);
-extern void discard_new_inode(struct inode *);
-extern unsigned int get_next_ino(void);
-extern void evict_inodes(struct super_block *sb);
+void unlock_new_inode(struct inode *);
+void discard_new_inode(struct inode *);
+unsigned int get_next_ino(void);
+void evict_inodes(struct super_block *sb);
void dump_mapping(const struct address_space *);
/*
@@ -3015,21 +3015,21 @@ int setattr_should_drop_sgid(struct mnt_idmap *idmap,
*/
#define alloc_inode_sb(_sb, _cache, _gfp) kmem_cache_alloc_lru(_cache, &_sb->s_inode_lru, _gfp)
-extern void __insert_inode_hash(struct inode *, u64 hashval);
+void __insert_inode_hash(struct inode *, u64 hashval);
static inline void insert_inode_hash(struct inode *inode)
{
__insert_inode_hash(inode, inode->i_ino);
}
-extern void __remove_inode_hash(struct inode *);
+void __remove_inode_hash(struct inode *);
static inline void remove_inode_hash(struct inode *inode)
{
if (!inode_unhashed(inode) && !hlist_fake(&inode->i_hash))
__remove_inode_hash(inode);
}
-extern void inode_sb_list_add(struct inode *inode);
-extern void inode_lru_list_add(struct inode *inode);
+void inode_sb_list_add(struct inode *inode);
+void inode_lru_list_add(struct inode *inode);
int generic_file_mmap(struct file *, struct vm_area_struct *);
int generic_file_mmap_prepare(struct vm_area_desc *desc);
---
base-commit: 1b626cfc5c2e16d02f0f7516b68b00aaa0a186cf
change-id: 20260306-iino-u64-18ba046632a0
Best regards,
--
Jeff Layton <jlayton@kernel.org>On Fri, Mar 06, 2026 at 08:27:01AM -0500, Jeff Layton wrote: > @@ -2951,26 +2951,26 @@ struct inode *iget5_locked(struct super_block *, u64, > struct inode *iget5_locked_rcu(struct super_block *, u64, > int (*test)(struct inode *, void *), > int (*set)(struct inode *, void *), void *); > -extern struct inode *iget_locked(struct super_block *, u64); > +struct inode *iget_locked(struct super_block *, u64); I think plain 'u64' deserves a name. I know some people get very upset when they see any unnamed parameter, but I don't think that you need to put "sb" in the first parameter. A u64 is non-obvious though; is it i_ino? Or hashval? > -extern struct inode *find_inode_nowait(struct super_block *, > +struct inode *find_inode_nowait(struct super_block *, > u64, > int (*match)(struct inode *, > u64, void *), > void *data); I think these need to be reflowed. Before they were aligned with the open bracket, and this demonstrates why that's a stupid convention. And the u64 needs a name. (other occurrences snipped)
On Sat, 2026-03-07 at 05:31 +0000, Matthew Wilcox wrote: > On Fri, Mar 06, 2026 at 08:27:01AM -0500, Jeff Layton wrote: > > @@ -2951,26 +2951,26 @@ struct inode *iget5_locked(struct super_block *, u64, > > struct inode *iget5_locked_rcu(struct super_block *, u64, > > int (*test)(struct inode *, void *), > > int (*set)(struct inode *, void *), void *); > > -extern struct inode *iget_locked(struct super_block *, u64); > > +struct inode *iget_locked(struct super_block *, u64); > > I think plain 'u64' deserves a name. I know some people get very > upset when they see any unnamed parameter, but I don't think that you > need to put "sb" in the first parameter. A u64 is non-obvious though; > is it i_ino? Or hashval? > > > -extern struct inode *find_inode_nowait(struct super_block *, > > +struct inode *find_inode_nowait(struct super_block *, > > u64, > > int (*match)(struct inode *, > > u64, void *), > > void *data); > > I think these need to be reflowed. Before they were aligned with the > open bracket, and this demonstrates why that's a stupid convention. > And the u64 needs a name. > > (other occurrences snipped) I toyed with changing those too, but there were quite a few. I'll plan to send an updated patch that names all of the parameters in anything that has them unnamed. That'll make checkpatch happy too. -- Jeff Layton <jlayton@kernel.org>
On Sat, Mar 07, 2026 at 05:31:24AM +0000, Matthew Wilcox wrote: > On Fri, Mar 06, 2026 at 08:27:01AM -0500, Jeff Layton wrote: > > @@ -2951,26 +2951,26 @@ struct inode *iget5_locked(struct super_block *, u64, > > struct inode *iget5_locked_rcu(struct super_block *, u64, > > int (*test)(struct inode *, void *), > > int (*set)(struct inode *, void *), void *); > > -extern struct inode *iget_locked(struct super_block *, u64); > > +struct inode *iget_locked(struct super_block *, u64); > > I think plain 'u64' deserves a name. I know some people get very > upset when they see any unnamed parameter, but I don't think that you > need to put "sb" in the first parameter. A u64 is non-obvious though; > is it i_ino? Or hashval? > > > -extern struct inode *find_inode_nowait(struct super_block *, > > +struct inode *find_inode_nowait(struct super_block *, > > u64, > > int (*match)(struct inode *, > > u64, void *), > > void *data); > > I think these need to be reflowed. Before they were aligned with the > open bracket, and this demonstrates why that's a stupid convention. > And the u64 needs a name. I think inode numbers ought to be their own typedef to make it *really obvious* when you're dealing with one, and was pretty sad to see "vfs: remove kino_t typedef and PRIino format macro" so soon after one was added. But our really excellent checkpatch tool says "do not add new typedefs" so instead everyone else has to be really smart about what "u64" represents when they see one, particularly because arithmetic is meaningless for this particular "u64". Yay. --D
On Fri, 2026-03-06 at 22:10 -0800, Darrick J. Wong wrote: > On Sat, Mar 07, 2026 at 05:31:24AM +0000, Matthew Wilcox wrote: > > On Fri, Mar 06, 2026 at 08:27:01AM -0500, Jeff Layton wrote: > > > @@ -2951,26 +2951,26 @@ struct inode *iget5_locked(struct super_block *, u64, > > > struct inode *iget5_locked_rcu(struct super_block *, u64, > > > int (*test)(struct inode *, void *), > > > int (*set)(struct inode *, void *), void *); > > > -extern struct inode *iget_locked(struct super_block *, u64); > > > +struct inode *iget_locked(struct super_block *, u64); > > > > I think plain 'u64' deserves a name. I know some people get very > > upset when they see any unnamed parameter, but I don't think that you > > need to put "sb" in the first parameter. A u64 is non-obvious though; > > is it i_ino? Or hashval? > > > > > -extern struct inode *find_inode_nowait(struct super_block *, > > > +struct inode *find_inode_nowait(struct super_block *, > > > u64, > > > int (*match)(struct inode *, > > > u64, void *), > > > void *data); > > > > I think these need to be reflowed. Before they were aligned with the > > open bracket, and this demonstrates why that's a stupid convention. > > And the u64 needs a name. > > I think inode numbers ought to be their own typedef to make it *really > obvious* when you're dealing with one, and was pretty sad to see "vfs: > remove kino_t typedef and PRIino format macro" so soon after one was > added. But our really excellent checkpatch tool says "do not add new > typedefs" so instead everyone else has to be really smart about what > "u64" represents when they see one, particularly because arithmetic is > meaningless for this particular "u64". > > Yay. > My take on typedefs is that they are mostly useful when you have a variable type that needs to be accessed in a particular way. For instance, I added a typedef for errseq_t since it's not just a plain integer (there are fields within it), and you need to use the correct functions to work with it. In this case though, it really is just a write-once read-many times bog-standard u64. I sort of wonder if we ought to label it const since inode numbers really shouldn't ever change after being established, but I didn't go that far. -- Jeff Layton <jlayton@kernel.org>
On Sat, Mar 07, 2026 at 01:00:15PM -0500, Jeff Layton wrote: > On Fri, 2026-03-06 at 22:10 -0800, Darrick J. Wong wrote: > > On Sat, Mar 07, 2026 at 05:31:24AM +0000, Matthew Wilcox wrote: > > > On Fri, Mar 06, 2026 at 08:27:01AM -0500, Jeff Layton wrote: > > > > @@ -2951,26 +2951,26 @@ struct inode *iget5_locked(struct super_block *, u64, > > > > struct inode *iget5_locked_rcu(struct super_block *, u64, > > > > int (*test)(struct inode *, void *), > > > > int (*set)(struct inode *, void *), void *); > > > > -extern struct inode *iget_locked(struct super_block *, u64); > > > > +struct inode *iget_locked(struct super_block *, u64); > > > > > > I think plain 'u64' deserves a name. I know some people get very > > > upset when they see any unnamed parameter, but I don't think that you > > > need to put "sb" in the first parameter. A u64 is non-obvious though; > > > is it i_ino? Or hashval? > > > > > > > -extern struct inode *find_inode_nowait(struct super_block *, > > > > +struct inode *find_inode_nowait(struct super_block *, > > > > u64, > > > > int (*match)(struct inode *, > > > > u64, void *), > > > > void *data); > > > > > > I think these need to be reflowed. Before they were aligned with the > > > open bracket, and this demonstrates why that's a stupid convention. > > > And the u64 needs a name. > > > > I think inode numbers ought to be their own typedef to make it *really > > obvious* when you're dealing with one, and was pretty sad to see "vfs: > > remove kino_t typedef and PRIino format macro" so soon after one was > > added. But our really excellent checkpatch tool says "do not add new > > typedefs" so instead everyone else has to be really smart about what > > "u64" represents when they see one, particularly because arithmetic is > > meaningless for this particular "u64". > > > > Yay. > > > > My take on typedefs is that they are mostly useful when you have a > variable type that needs to be accessed in a particular way. For > instance, I added a typedef for errseq_t since it's not just a plain > integer (there are fields within it), and you need to use the correct > functions to work with it. > > In this case though, it really is just a write-once read-many times > bog-standard u64. I sort of wonder if we ought to label it const since > inode numbers really shouldn't ever change after being established, but > I didn't go that far. I would be very interested in seeing any filesystem that changes inode numbers...
On Fri, Mar 06, 2026 at 08:27:01AM -0500, Jeff Layton wrote: > Christoph says, in response to one of the patches in the i_ino widening > series, which changes the prototype of several functions in fs.h: > > "Can you please drop all these pointless externs while you're at it?" > > Remove extern keyword from functions touched by that patch (and a few > that happened to be nearby). > Is there a reason to not straight up whack the keyword from *all* funcs in the file?
On Fri, 2026-03-06 at 23:17 +0100, Mateusz Guzik wrote: > On Fri, Mar 06, 2026 at 08:27:01AM -0500, Jeff Layton wrote: > > Christoph says, in response to one of the patches in the i_ino widening > > series, which changes the prototype of several functions in fs.h: > > > > "Can you please drop all these pointless externs while you're at it?" > > > > Remove extern keyword from functions touched by that patch (and a few > > that happened to be nearby). > > > > Is there a reason to not straight up whack the keyword from *all* funcs in > the file? We could but that makes merge conflicts for backporters. In this case I'm already touching these function prototypes (well, most of them anyway. I took some liberties with adjacent declarations). Doing this in a limited fashion shouldn't be a huge burden on people backporting since we're already changing this area of the file. Certainly we could do more cleanup in there, but that makes it trickier to backport later patches that touch fs.h to older kernels. Ultimately, it's a judgement call though. A one-time "let's fix all of the warts in fs.h" patch is not out of the question, IMO. -- Jeff Layton <jlayton@kernel.org>
© 2016 - 2026 Red Hat, Inc.