[PATCH 4/8] fs: introduce simple_new_inode
Greg Kroah-Hartman
gregkh at linuxfoundation.org
Tue Apr 14 23:01:40 AEST 2020
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?
> --- 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
More information about the Linuxppc-dev
mailing list