Re: [PATCH 4/8] fs: introduce simple_new_inode
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: 2020-04-14 13:01:45
Also in:
autofs, dri-devel, linux-mm, ocfs2-devel
On Tue, Apr 14, 2020 at 02:42:58PM +0200, Emanuele Giuseppe Esposito wrote:
It is a common special case for new_inode to initialize the time to the current time and the inode to get_next_ino(). Introduce a core function that does it and use it throughout Linux.
Shouldn't this just be called new_inode_current_time()? How is anyone going to remember what simple_new_inode() does to the inode structure?
quoted hunk ↗ jump to hunk
--- a/fs/libfs.c +++ b/fs/libfs.c@@ -595,6 +595,18 @@ int simple_write_end(struct file *file, struct address_space *mapping, } EXPORT_SYMBOL(simple_write_end); +struct inode *simple_new_inode(struct super_block *sb) +{ + struct inode *inode = new_inode(sb); + if (inode) { + inode->i_ino = get_next_ino(); + inode->i_atime = inode->i_mtime = + inode->i_ctime = current_time(inode); + } + return inode; +} +EXPORT_SYMBOL(simple_new_inode);
No kernel doc explaining that get_next_ino() is called already? Please document new global functions like this so we have a chance to know how to use them. Also, it is almost always easier to introduce a common function, get it merged, and _THEN_ send out cleanup functions to all of the different subsystems to convert over to it. Yes, it takes longer, but it makes it possible to do this in a way that can be reviewed properly, unlike this patch series :( thanks, greg k-h